dev@javaserverfaces.java.net

Seeking Review: backwards compat: empty string nav case (was: Re: [jsr-314-open] Handling of "" vs null action outcomes)

From: Ed Burns <Ed.Burns_at_Sun.COM>
Date: Tue, 16 Feb 2010 07:33:29 -0800

>>>>> On Tue, 16 Feb 2010 05:51:54 -0800, Ed Burns <ed.burns_at_sun.com> said:

>>>>> On Mon, 15 Feb 2010 15:54:06 -0600, Jason Lee <jason_at_steeplesoft.com> said:
JL> On 2/15/10 3:49 PM, Andy Schwartz wrote:

AS> FWIW, my feeling is that, while returning null is a reasonable
AS> workaround, there is no way that our unit test is not the only code
AS> out there that is returning empty action outcomes and expecting to
AS> stay on the same page. I would like to see this behavior preserved not
AS> so that we can revert our unit test to its previous behavior (I am
AS> fine with our fix), but because the current behavior may break other
AS> users as they upgrade to JSF2.

JL> I know in the apps I've worked on whether I return an empty String or
JL> null depends on the mood I'm in. :) I think we should clarify the spec
JL> to preserve the 1.2 behavior.

EB> I agree completely.

EB> I have added this to the changelog and am implementing it presently.
EB> It's issue C063 in the Changelog.

Can someone please review this? I'll write the automated test is
parallel.

Ed


Clarify that, in the case of navigation actions, an empty string should
be treated the same way as null: stay on the same page.

http://wiki.jcp.org/wiki/index.php?page=JSF+2.0+Rev+A+Change+Log

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

- Within the case for implicit navigation, if there is no match, and the
  outcome is a zero length string, set the outcome to null so the normal
  processing of "null outcome" is taken.

M jsf-ri/systest/src/com/sun/faces/systest/model/TestBean.java
A jsf-ri/systest/web/implicitnav/implicitNavEmptyString.xhtml

- Assume the test is forthcoming.


SECTION: Diffs
----------------------------
Index: jsf-ri/src/com/sun/faces/application/NavigationHandlerImpl.java
===================================================================
--- jsf-ri/src/com/sun/faces/application/NavigationHandlerImpl.java (revision 8329)
+++ jsf-ri/src/com/sun/faces/application/NavigationHandlerImpl.java (working copy)
 -286,6 +286,11 @@
         // If the navigation rules do not have a match...
         if (caseStruct == null && outcome != null && viewId != null) {
             caseStruct = findImplicitMatch(ctx, viewId, fromAction, outcome);
+ // Treat empty string equivalent to null outcome. JSF 2.0 Rev a
+ // Changelog issue C063.
+ if (caseStruct == null && 0 == outcome.length()) {
+ outcome = null;
+ }
         }
 
         // no navigation case fo
Index: jsf-ri/systest/src/com/sun/faces/systest/model/TestBean.java
===================================================================
--- jsf-ri/systest/src/com/sun/faces/systest/model/TestBean.java (revision 8329)
+++ jsf-ri/systest/src/com/sun/faces/systest/model/TestBean.java (working copy)
 -914,4 +914,8 @@
         this.color = color;
     }
 
+ public String emptyStringAction() {
+ return null;
+ }
+
 }
Index: jsf-ri/systest/web/implicitnav/implicitNavEmptyString.xhtml
===================================================================
--- jsf-ri/systest/web/implicitnav/implicitNavEmptyString.xhtml (revision 0)
+++ jsf-ri/systest/web/implicitnav/implicitNavEmptyString.xhtml (revision 0)
 -0,0 +1,57 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
+
+ Copyright 1997-2010 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.
+
+-->
+<!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" xml:lang="en" lang="en"
+ xmlns:h="http://java.sun.com/jsf/html">
+<h:head>
+ <title>Implicit Navigation with empty string action</title>
+</h:head>
+<h:body>
+
+ <h:form prependId="false">
+
+ <p><h:commandButton action="#{test1.emptyStringAction}" value="stay here" /></p>
+
+
+ </h:form>
+
+</h:body>
+</html>


SECTION: New Files
----------------------------
SEE ATTACHMENTS

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