dev@javaserverfaces.java.net

Re: Seeking Review: 438, 362, 456

From: Ryan Lubke <Ryan.Lubke_at_Sun.COM>
Date: Mon, 22 Sep 2008 08:30:09 -0700

Questions inline...

Ed Burns wrote:
> Sorry for glomming these together, but I did them on the plane without
> net access.
>
> Issue: 438-NewExternalContextMethods, 362-FacesContext.getMessageList,
> 456-saveAttachedState.
>
> SECTION: API Changes
>
> M jsf-api/src/javax/faces/context/FacesContext.java
>
> - Expose getMessageList() for EL.
>
> M jsf-api/src/javax/faces/context/ExternalContext.java
>
> - New methods on external context, requested by Matzew.
>
> M jsf-api/src/javax/faces/component/UIComponentBase.java
>
>
> - modify saveAttachedState to support Collection instead of List.
>
> A jsf-api/test/javax/faces/component/UIComponentBaseAttachedStateTestCase.java
> A jsf-ri/systest/web/message05.jsp
>
> - New testcase, needs more work.
>
Which one needs more work?
> SECTION: Mojarra changes
>
> M jsf-ri/src/com/sun/faces/context/FacesContextImpl.java
> M jsf-ri/src/com/sun/faces/config/InitFacesContext.java
>
> - impl for 362
>
> M jsf-ri/src/com/sun/faces/context/ExternalContextImpl.java
> M jsf-ri/test/com/sun/faces/config/StoreServletContext.java
>
> - impl for 438
>
> M lib/jsf-extensions-test-time.jar
>
> - new test time.
>
> SECTION: Diffs
>
> Index: jsf-api/test/javax/faces/component/UIComponentBaseAttachedStateTestCase.java
> ===================================================================
> --- jsf-api/test/javax/faces/component/UIComponentBaseAttachedStateTestCase.java (revision 0)
> +++ jsf-api/test/javax/faces/component/UIComponentBaseAttachedStateTestCase.java (revision 0)
> @@ -0,0 +1,75 @@
> +/*
> + * To change this template, choose Tools | Templates
> + * and open the template in the editor.
> + */
> +
> +package javax.faces.component;
> +
> +import com.sun.faces.mock.MockFacesContext;
> +import java.util.HashSet;
> +import java.util.Set;
> +import java.util.Stack;
> +import javax.faces.event.ValueChangeListener;
> +import junit.framework.Test;
> +import junit.framework.TestCase;
> +import junit.framework.TestSuite;
> +
> +/**
> + *
> + * @author edburns
>
> + */
> +public class UIComponentBaseAttachedStateTestCase extends TestCase {
> +
> + private UIComponentBase component;
> + private MockFacesContext facesContext = null;
> +
> +
> + public UIComponentBaseAttachedStateTestCase(String arg0) {
> + super(arg0);
> + }
> +
> + // Return the tests included in this test case.
> + public static Test suite() {
> + return (new TestSuite(UIComponentBaseAttachedStateTestCase.class));
> + }
> +
> + public void setUp() throws Exception {
> +
> + // Set up the component under test
> + super.setUp();
> + component = new UIOutput();
> + facesContext = new MockFacesContext();
> +
> + }
> +
> + public void testAttachedObjectsSet() throws Exception {
> + Set<ValueChangeListener> returnedAttachedObjects = null,
> + attachedObjects = new HashSet<ValueChangeListener>();
> + ValueChangeListener toAdd = new TestValueChangeListener();
> + attachedObjects.add(toAdd);
> + toAdd = new TestValueChangeListener();
> + attachedObjects.add(toAdd);
> + toAdd = new TestValueChangeListener();
> + attachedObjects.add(toAdd);
> + Object result = UIComponentBase.saveAttachedState(facesContext, attachedObjects);
> + returnedAttachedObjects = (Set<ValueChangeListener>)
> + UIComponentBase.restoreAttachedState(facesContext, result);
> +
> + }
> +
> + public void testAttachedObjectsStack() throws Exception {
> + Stack<ValueChangeListener> returnedAttachedObjects = null,
> + attachedObjects = new Stack<ValueChangeListener>();
> + ValueChangeListener toAdd = new TestValueChangeListener();
> + attachedObjects.add(toAdd);
> + toAdd = new TestValueChangeListener();
> + attachedObjects.add(toAdd);
> + toAdd = new TestValueChangeListener();
> + attachedObjects.add(toAdd);
> + Object result = UIComponentBase.saveAttachedState(facesContext, attachedObjects);
> + returnedAttachedObjects = (Stack<ValueChangeListener>)
> + UIComponentBase.restoreAttachedState(facesContext, result);
> +
> + }
> +
> +}
> Index: jsf-api/src/javax/faces/context/FacesContext.java
> ===================================================================
> --- jsf-api/src/javax/faces/context/FacesContext.java (revision 5455)
> +++ jsf-api/src/javax/faces/context/FacesContext.java (working copy)
> @@ -329,8 +329,54 @@
> */
> public abstract Iterator<FacesMessage> getMessages();
>
> + /**
> + * <p class="changed_added_2_0">Like {_at_link getMessages}, but
> + * returns an unmodifiable <code>List&lt;FacesMessage&gt;</code>,
> + * enabling use from EL expressions.</p>
> + *
> + * <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>
> + *
> + * @throws IllegalStateException if this method is called after
> + * this instance has been released
> + * @since 2.0
> + */
>
> + public List<FacesMessage> getMessageList() {
> + if (defaultFacesContext != null) {
> + return defaultFacesContext.getMessageList();
> + }
> + throw new UnsupportedOperationException();
> + }
> +
> /**
> + * <p class="changed_added_2_0">Like {_at_link
> + * getMessages(java.lang.String)}, but returns an unmodifiable
> + * <code>List&lt;FacesMessage&gt;</code>.</p>
> + *
> + * <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>
> + *
> + * @throws IllegalStateException if this method is called after
> + * this instance has been released
> + * @since 2.0
> + */
> +
> + public List<FacesMessage> getMessageList(String clientId) {
> + if (defaultFacesContext != null) {
> + return defaultFacesContext.getMessageList(clientId);
> + }
> + throw new UnsupportedOperationException();
> + }
> +
> +
> +
> +
> + /**
> * <p>Return an <code>Iterator</code> over the {_at_link javax.faces.application.FacesMessage}s that
> * have been queued that are associated with the specified client identifier
> * (if <code>clientId</code> is not <code>null</code>), or over the
>

I'm curios as to why these new methods are returning unmodifiable
Lists? The Iterator version of these methods support modifications.
Seems odd that one can be modified while the other cannot.

> Index: jsf-api/src/javax/faces/context/ExternalContext.java
> ===================================================================
> --- jsf-api/src/javax/faces/context/ExternalContext.java (revision 5455)
> +++ jsf-api/src/javax/faces/context/ExternalContext.java (working copy)
> @@ -433,7 +433,37 @@
> */
> public abstract Object getContext();
>
> + /**
> + *
> + * <p class="changed_added_2_0">Return the name of the container
> + * context for this application. </p>
> + *
> + * <p class="changed_added_2_0">Return the result of calling
> + * <code>getServletContextName()</code> on the
> + * <code>ServletContext</code> instance for this application. It is
> + * valid to call this method during application startup.</p>
> + *
> + * <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>
> + *
> + *
> + */
>
> + public String getContextName() {
> +
> + if (defaultExternalContext != null) {
> + return defaultExternalContext.getContextName();
> + }
> +
> + throw new UnsupportedOperationException();
> +
> + }
> +
> +
> +
> +
> /**
> * <p><span class="changed_modified_2_0">Return</span> the value of
> * the specified application initialization parameter (if any).</p>
> @@ -926,7 +956,31 @@
> }
>
> /**
> + * <p class="changed_added_2_0">Return the result
> + * of calling <code>getContentLenth()</code> on the
> + * <code>ServletRequest</code> instance for this request.</p>
> *
> + * <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>
> + *
> + * @since 2.0
> + */
> +
> + public int getRequestContentLength() {
> +
> + if (defaultExternalContext != null) {
> + return defaultExternalContext.getRequestContentLength();
> + }
> +
> + throw new UnsupportedOperationException();
> +
> + }
> +
> +
> + /**
> + *
> * <p>Returns the name of the character encoding (MIME charset) used for
> * the body sent in this response. </p>
> *
> Index: jsf-api/src/javax/faces/component/UIComponentBase.java
> ===================================================================
> --- jsf-api/src/javax/faces/component/UIComponentBase.java (revision 5455)
> +++ jsf-api/src/javax/faces/component/UIComponentBase.java (working copy)
> @@ -54,12 +54,14 @@
> import java.io.Serializable;
> import java.io.ObjectOutputStream;
> import java.io.ObjectInputStream;
> +import java.lang.Object;
> import java.lang.reflect.InvocationTargetException;
> import java.lang.reflect.Method;
> import java.util.AbstractCollection;
> import java.util.AbstractSet;
> import java.util.ArrayList;
> import java.util.Collection;
> +import java.util.Collection;
> import java.util.Collections;
> import java.util.HashMap;
> import java.util.Iterator;
> @@ -73,9 +75,6 @@
> import java.util.logging.Logger;
> import javax.faces.FactoryFinder;
> import javax.faces.application.Resource;
> -import javax.faces.application.ResourceDependencies;
> -import javax.faces.application.ResourceDependency;
> -import javax.faces.application.ResourceHandler;
> import javax.faces.event.AfterAddToParentEvent;
> import javax.faces.event.BeforeRenderEvent;
> import javax.faces.event.PhaseId;
> @@ -1410,10 +1409,11 @@
> }
> Object result;
>
> - if (attachedObject instanceof List) {
> - List attachedList = (List) attachedObject;
> - List<StateHolderSaver> resultList = new ArrayList<StateHolderSaver>(attachedList.size());
> - Iterator listIter = attachedList.iterator();
> + if (attachedObject instanceof Collection) {
> + Collection attachedCollection = (Collection) attachedObject;
> + List<StateHolderSaver> resultList = new ArrayList<StateHolderSaver>(attachedCollection.size() + 1);
> + resultList.add(new StateHolderSaver(context, attachedCollection.getClass()));
> + Iterator listIter = attachedCollection.iterator();
> Object cur;
> while (listIter.hasNext()) {
> if (null != (cur = listIter.next())) {
> @@ -1463,16 +1463,25 @@
> Object result;
>
> if (stateObj instanceof List) {
> - List stateList = (List) stateObj;
> - List<Object> retList = new ArrayList<Object>(stateList.size());
> + List<StateHolderSaver> stateList = (List<StateHolderSaver>) stateObj;
> + Collection<Object> retCollection = null;
> + StateHolderSaver collectionSaver = stateList.remove(0);
> + Class collectionClass = (Class) collectionSaver.restore(context);
> + try {
> + retCollection = (Collection<Object>) collectionClass.newInstance();'
>
> + }
> + catch (Exception e) {
> + throw new IllegalStateException("Unknown object type");
> + }
> +
> for (Object item : stateList) {
> try {
> - retList.add(((StateHolderSaver) item).restore(context));
> + retCollection.add(((StateHolderSaver) item).restore(context));
> } catch (ClassCastException cce) {
> throw new IllegalStateException("Unknown object type");
> }
> }
> - result = retList;
> + result = retCollection;
> } else if (stateObj instanceof StateHolderSaver) {
> StateHolderSaver saver = (StateHolderSaver) stateObj;
> result = saver.restore(context);
> Index: jsf-ri/test/com/sun/faces/config/StoreServletContext.java
> ===================================================================
> --- jsf-ri/test/com/sun/faces/config/StoreServletContext.java (revision 5455)
> +++ jsf-ri/test/com/sun/faces/config/StoreServletContext.java (working copy)
> @@ -125,6 +125,11 @@
> return servletContext;
> }
>
> + public String getContextName() {
> + return servletContext.getServletContextName();
> + }
> +
>
> +
> public String getInitParameter(String name) {
> return null;
> }
> @@ -222,6 +227,10 @@
> return null;
> }
>
> + public int getRequestContentLength() {
> + return -1;
> + }
> +
> public String getResponseContentType() {
> return null;
> }
> Index: jsf-ri/src/com/sun/faces/context/FacesContextImpl.java
> ===================================================================
> --- jsf-ri/src/com/sun/faces/context/FacesContextImpl.java (revision 5455)
> +++ jsf-ri/src/com/sun/faces/context/FacesContextImpl.java (working copy)
> @@ -369,7 +369,36 @@
> return result;
> }
>
> + public List<FacesMessage> getMessageList() {
> + List<FacesMessage> results = null;
> + if (null == componentMessageLists) {
> + results = Collections.EMPTY_LIST;
> + } else {
> + results = new ArrayList<FacesMessage>();
> + Iterator clientIds = componentMessageLists.keySet().iterator();
> + while (clientIds.hasNext()) {
> + String clientId = (String) clientIds.next();
> + results.addAll((List<FacesMessage>) componentMessageLists.get(clientId));
> + }
> + }
>
> + return Collections.unmodifiableList(results);
> + }
> +
> + public List<FacesMessage> getMessageList(String clientId) {
> + List<FacesMessage> result = null;
> + if (null == componentMessageLists) {
> + result = Collections.EMPTY_LIST;
> + } else {
> + componentMessageLists.get(clientId);
> + if (result == null) {
> + result = Collections.EMPTY_LIST;
> + }
> + }
> + return Collections.unmodifiableList(result);
> + }
> +
> +
> /**
> * @see javax.faces.context.FacesContext#getMessages()
> */
> @@ -380,7 +409,7 @@
> return (emptyList.iterator());
> }
>
> - //Clear set of clientIds from pending display messages list.
> + //Clear set of clientIds from pending display messages result.
> if (RequestStateManager.containsKey(this, RequestStateManager.CLIENT_ID_MESSAGES_NOT_DISPLAYED)) {
> Set pendingClientIds = (Set)
> RequestStateManager.get(this, RequestStateManager.CLIENT_ID_MESSAGES_NOT_DISPLAYED);
> @@ -401,7 +430,7 @@
> public Iterator<FacesMessage> getMessages(String clientId) {
> assertNotReleased();
>
> - //remove client id from pending display messages list.
> + //remove client id from pending display messages result.
> Set pendingClientIds = (Set)
> RequestStateManager.get(this, RequestStateManager.CLIENT_ID_MESSAGES_NOT_DISPLAYED);
> if (pendingClientIds != null && !pendingClientIds.isEmpty()) {
> Index: jsf-ri/src/com/sun/faces/context/ExternalContextImpl.java
> ===================================================================
> --- jsf-ri/src/com/sun/faces/context/ExternalContextImpl.java (revision 5455)
> +++ jsf-ri/src/com/sun/faces/context/ExternalContextImpl.java (working copy)
> @@ -153,7 +153,15 @@
> return this.servletContext;
> }
>
> + /**
> + * @see javax.faces.context.ExternalContext#getContextName()
> + */
> + public String getContextName() {
> + return this.servletContext.getServletContextName();
> + }
>
> +
> +
> /**
> * @see javax.faces.context.ExternalContext#getRequest()
> */
> @@ -415,7 +423,16 @@
> return (request.getContentType());
> }
>
> + /**
> + * @see javax.faces.context.ExternalContext#getRequestContentLength()
> + */
> + @Override
> + public int getRequestContentLength() {
> + return (request.getContentLength());
> + }
>
> +
> +
> /**
> * @see javax.faces.context.ExternalContext#getResponseCharacterEncoding()
> */
> Index: jsf-ri/src/com/sun/faces/config/InitFacesContext.java
> ===================================================================
> --- jsf-ri/src/com/sun/faces/config/InitFacesContext.java (revision 5455)
> +++ jsf-ri/src/com/sun/faces/config/InitFacesContext.java (working copy)
> @@ -127,6 +127,17 @@
> return getMessages();
> }
>
> + public List<FacesMessage> getMessageList() {
> + return Collections.EMPTY_LIST;
> + }
> +
> +
> + public List<FacesMessage> getMessageList(String clientId) {
> + return Collections.EMPTY_LIST;
> + }
> +
> +
> +
> public RenderKit getRenderKit() {
> return null;
> }
> @@ -231,6 +242,12 @@
> return servletContext;
> }
>
> + public String getContextName() {
> + return servletContext.getServletContextName();
> + }
> +
> +
> +
> public String getInitParameter(String name) {
> return servletContext.getInitParameter(name);
> }
> @@ -321,6 +338,10 @@
> return null;
> }
>
> + public int getRequestContentLength() {
> + return -1;
> + }
> +
> public URL getResource(String path) throws MalformedURLException {
> return servletContext.getResource(path);
> }
> Index: jsf-ri/systest/web/message05.jsp
> ===================================================================
> --- jsf-ri/systest/web/message05.jsp (revision 0)
> +++ jsf-ri/systest/web/message05.jsp (revision 0)
> @@ -0,0 +1,89 @@
> +<%--
> + DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
> +
> + Copyright 1997-2007 Sun Microsystems, Inc. All rights reserved.
> +
> + The contents of this file are subject to the terms of either the GNU
> + General Public License Version 2 only ("GPL") or the Common Development
> + and Distribution License("CDDL") (collectively, the "License"). You
> + may not use this file except in compliance with the License. You can obtain
> + a copy of the License at https://glassfish.dev.java.net/public/CDDL+GPL.html
> + or glassfish/bootstrap/legal/LICENSE.txt. See the License for the specific
> + language governing permissions and limitations under the License.
> +
> + When distributing the software, include this License Header Notice in each
> + file and include the License file at glassfish/bootstrap/legal/LICENSE.txt.
> + Sun designates this particular file as subject to the "Classpath" exception
> + as provided by Sun in the GPL Version 2 section of the License file that
> + accompanied this code. If applicable, add the following below the License
> + Header, with the fields enclosed by brackets [] replaced by your own
> + identifying information: "Portions Copyrighted [year]
> + [name of copyright owner]"
> +
> + Contributor(s):
> +
> + If you wish your version of this file to be governed by only the CDDL or
> + only the GPL Version 2, indicate your decision by adding "[Contributor]
> + elects to include this software in this distribution under the [CDDL or GPL
> + Version 2] license." If you don't indicate a single choice of license, a
> + recipient has the option to distribute your version of this file under
> + either the CDDL, the GPL Version 2 or to extend the choice of license to
> + its licensees as provided above. However, if you add GPL Version 2 code
> + and therefore, elected the GPL Version 2 license, then the option applies
> + only if the new code is made subject to such option by the copyright
> + holder.
> +--%>
> +
> +<!--
> + Copyright 2004 Sun Microsystems, Inc. All rights reserved.
> + SUN PROPRIETARY/CONFIDENTIAL. Use is subject to license terms.
> +-->
> +<%@ page contentType="text/html" language="java" %>
> +<%@ taglib prefix="c" uri="http://java.sun.com/jstl/core" %>
> +<%@ taglib prefix="f" uri="http://java.sun.com/jsf/core" %>
> +<%@ taglib prefix="h" uri="http://java.sun.com/jsf/html" %>
> +
> +<f:view>
> +<head>
> +<title>Test FacesContext.getMessageList()</title>
> +</head>
> +<body>
> +
> +<h:form id="form" prependId="false">
> +<p>
> +
> +Enter a non-number string here:
> +<h:inputText id="booleanTextField" value="#{test1.intProperty}" />
> +
> +</p>
> +
> +<p>Here's the h:messages tag</p>
> +
> +<p><h:messages /></p>
> +
> +<p><h:outputText value="#{facesContext.messageList} "/></p>
> +
> +<p>Here's the same using c:forEach with the new
> +FacesContext.getMessageList() method. PENDING: why doesn't this
> +work?</p>
> +
> +<table border="1">
> +
> +<c:forEach var="message" items="#{facesContext.messageList}">
> +
> + <tr>
> +
> + <td>summary: <h:outputText value="#{message.summary}" /></td>
> + <td>detail: <h:outputText value="#{message.detail}" /></td>
> + <td>severity: <h:outputText value="#{message.severity}" /></td>
> +
> + </tr>
> +
> +</c:forEach>
> +
> +<p><h:commandButton id="reload" value="Reload" /></p>
> +
> +</h:form>
> +
> +</body>
> +</f:view>
> Index: lib/jsf-extensions-test-time.jar
> ===================================================================
> Cannot display: file marked as a binary type.
> svn:mime-type = application/octet-stream
>