dev@glassfish.java.net

Re: [NEW] Seeking Review: Small EL Impl changes

From: Ken Paulsen <ken.paulsen_at_Sun.COM>
Date: Thu, 11 Aug 2005 16:01:56 -0700

You may want to consider simplifying the API by having "ELException"
utilize the ELUtil class. Then you could just pass in the key:
"setPropertyFailed" and the object[].

Also, I would think developers may want to customize these messages. Do
you have a mechanism for doing this? Perhaps a ResourceBundleFactory?

Ken


Ed Burns wrote:

>This change-bundle enables localized messages to be generated when an
>error condition arises from deep within the bowels of the EL.
>
>It relies on a convention I'm currently trying to make a standard in the
>webtier-alignment Expert Group, but even if it doesn't pass, I still
>think it's a useful implementation feature.
>
>For example, without this feature, if you're using jsf on a text field
>bound to a primitive typed javaBeans property, and you submit a form
>with no value in the text field, you get the message:
>
> java.lang.IllegalArgumentException.
>
>With this change, you instead get:
>
> Can't set property intProperty of type int on class com.foo.Foo to value
> null.
>
>Of course, there are more strings to localize, and we'll probably want
>to change the bundle name to be something glassfish specific.
>
>Anyhow, this is being tracked under bug
>
>https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=167
>
>I include the change-bundle here for your convenience. Disregard the
>Faces changes.
>
>Ed
>
>
>Issue: javaserverfaces 167
>
>SECTION: EL API Changes
>
>M src/jsr245/src/share/javax/el/BeanELResolver.java
>
>- modify to use new ELUtil.getExceptionMessageString() method.
>
>A src/jsr245/src/share/javax/el/ELUtil.java
>
>- new package private class, currently offering one method
> getExceptionMessageString(). See SECTION: EL API New Files
>
>A src/jsr245/src/share/javax/el/Messages.properties
>
>- Localized messages for EL problems. Currently only one message, but
> many more will follow.
>
>SECTION: JSF RI Changes
>
>M jsf-ri/src/com/sun/faces/context/FacesContextImpl.java
>
>- populate the ELContext with the current Locale.
>
>SECTION: EL API Diffs
>
>Index: src/jsr245/src/share/javax/el/BeanELResolver.java
>===================================================================
>RCS file: /cvs/glassfish/servlet-api/src/jsr245/src/share/javax/el/BeanELResolver.java,v
>retrieving revision 1.2
>diff -u -r1.2 BeanELResolver.java
>--- src/jsr245/src/share/javax/el/BeanELResolver.java 22 Jun 2005 18:34:11 -0000 1.2
>+++ src/jsr245/src/share/javax/el/BeanELResolver.java 11 Aug 2005 21:19:10 -0000
>@@ -289,6 +289,7 @@
> String sProperty = property.toString();
> BeanInfo info = null;
> Class clazz = base.getClass();
>+ Class type = null;
> try {
> info = Introspector.getBeanInfo(clazz);
> if (info == null) {
>@@ -299,7 +300,7 @@
> for (int i = 0; i < pd.length; i++) {
> if (pd[i].getName().equals(sProperty)) {
> Method method = getMethod(clazz, pd[i].getWriteMethod());
>- Class type = pd[i].getPropertyType();
>+ type = pd[i].getPropertyType();
> if (method == null) {
> throw new PropertyNotWritableException("Property '"
> + sProperty
>@@ -319,7 +320,14 @@
> } catch (InvocationTargetException ite) {
> throw new ELException(ite.getCause());
> } catch (Exception ex) {
>- throw new ELException(ex);
>+ if (null == val) {
>+ val = "null";
>+ }
>+ Object [] params = { sProperty,
>+ type.getName(), clazz.getName(), val };
>+ String message = ELUtil.getExceptionMessageString(context,
>+ "setPropertyFailed", params);
>+ throw new ELException(message, ex);
> }
> throw new PropertyNotFoundException("Property '"
> + sProperty
>
>SECTION: JSF RI Diffs
>
>Index: jsf-ri/src/com/sun/faces/context/FacesContextImpl.java
>===================================================================
>RCS file: /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/context/FacesContextImpl.java,v
>retrieving revision 1.70
>diff -u -r1.70 FacesContextImpl.java
>--- jsf-ri/src/com/sun/faces/context/FacesContextImpl.java 18 Jul 2005 19:28:53 -0000 1.70
>+++ jsf-ri/src/com/sun/faces/context/FacesContextImpl.java 11 Aug 2005 21:23:29 -0000
>@@ -34,6 +34,7 @@
> import java.util.Collections;
> import java.util.HashMap;
> import java.util.Iterator;
>+import java.util.Locale;
> import java.util.List;
> import java.util.Map;
> import java.util.LinkedHashMap;
>@@ -142,6 +143,7 @@
> if (elContext == null) {
> elContext = new ELContextImpl(getApplication().getELResolver());
> elContext.putContext(FacesContext.class, this);
>+ elContext.putContext(Locale.class, this.getViewRoot().getLocale());
> }
> return elContext;
> }
>
>SECTION: EL API New Files
>
>8<---------------------------- ELUtil.java
>/*
> * $Id: ELUtil.java,v 1.1 2005/05/05 20:51:22 edburns Exp $
> */
>/*
> * The contents of this file are subject to the terms
> * of the Common Development and Distribution License
> * (the "License"). You may not use this file except
> * in compliance with the License.
> *
> * You can obtain a copy of the license at
> * legal/CDDLv1.0.txt or
> * https://javaserverfaces.dev.java.net/CDDL.html.
> * See the License for the specific language governing
> * permissions and limitations under the License.
> *
> * When distributing Covered Code, include this CDDL
> * HEADER in each file and include the License file at
> * legal/CDDLv1.0.txt. If applicable,
> * add the following below this CDDL HEADER, with the
> * fields enclosed by brackets "[]" replaced with your
> * own identifying information: Portions Copyright [yyyy]
> * [name of copyright owner]
> */
>/*
> * Portions Copyright 2005 Sun Microsystems.
> */
>
>package javax.el;
>
>import java.text.MessageFormat;
>import java.util.HashMap;
>import java.util.Locale;
>import java.util.Map;
>import java.util.MissingResourceException;
>import java.util.ResourceBundle;
>
>/**
> *
> * <p>Utility methods for this portion of the EL implementation</p>
> *
> * <p>Methods on this class use a Map instance stored in ThreadLocal storage
> * to minimize the performance impact on operations that take place multiple
> * times on a single Thread. The keys and values of the Map
> * are implementation private.</p>
> *
> * @author edburns
> */
>class ELUtil {
>
> /**
> * <p>This class may not be constructed.</p>
> */
>
> private ELUtil() {
> }
>
> /**
> * <p>The <code>ThreadLocal</code> variable used to record the
> * {_at_link FacesContext} instance for each processing thread.</p>
> */
> private static ThreadLocal instance = new ThreadLocal() {
> protected Object initialValue() { return (null); }
> };
>
> /**
> * @return a Map stored in ThreadLocal storage. This may
> * be used by methods of this class to minimize the performance
> * impact for operations that may take place multiple times on a given
> * Thread instance.
> */
>
> private static Map getCurrentInstance() {
> Map result = (Map) instance.get();
> if (null == result) {
> result = new HashMap();
> setCurrentInstance(result);
> }
> return result;
>
> }
>
> /**
> * <p>Replace the Map with the argument context.</p>
> *
> * @param context, the Map to be stored in ThreadLocal storage.
> */
>
> private static void setCurrentInstance(Map context) {
>
> instance.set(context);
>
> }
>
> /*
> * @return the current thread's ContextClassLoader, if present.
> * If not found, return the fallbackClass's ClassLoader.
> *
> * @param fallbackClass In the event this Thread doesn't
> * have a contextClassLoader, use the fallbackClass's ClassLoader.
> *
> */
>
> private static ClassLoader getCurrentLoader(Object fallbackClass) {
> ClassLoader loader =
> Thread.currentThread().getContextClassLoader();
> if (loader == null) {
> loader = fallbackClass.getClass().getClassLoader();
> }
> return loader;
> }
>
> /*
> * <p>Convenience method, calls through to
> * {_at_link #getExceptionMessageString(javax.el.ELContext,java.lang.String,Object []).
> * </p>
> *
> * @param context the ELContext from which the Locale for this message
> * is extracted.
> *
> * @param messageId the messageId String in the ResourceBundle
> *
> * @return a localized String for the argument messageId
> */
>
> public static String getExceptionMessageString(ELContext context, String messageId) {
> return getExceptionMessageString(context, messageId, null);
> }
>
> /*
> * <p>Return a Localized message String suitable for use as an Exception message.
> * Examine the argument <code>context</code> for a <code>Locale</code>. If
> * not present, use <code>Locale.getDefault()</code>. Load the
> * <code>ResourceBundle</code> "javax.el.Messages" using that locale. Get
> * the message string for argument <code>messageId</code>. If not found
> * return "Missing Resource in EL implementation ??? messageId ???"
> * with messageId substituted with the runtime
> * value of argument <code>messageId</code>. If found, and argument
> * <code>params</code> is non-null, format the message using the
> * params. If formatting fails, return a sensible message including
> * the <code>messageId</code>. If argument <code>params</code> is
> * <code>null</code>, skip formatting and return the message directly, otherwise
> * return the formatted message.</p>
> *
> * @param context the ELContext from which the Locale for this message
> * is extracted.
> *
> * @param messageId the messageId String in the ResourceBundle
> *
> * @param params parameters to the message
> *
> * @return a localized String for the argument messageId
> */
>
> public static String getExceptionMessageString(ELContext context,
> String messageId,
> Object [] params) {
> String result = "";
> Object localeObject = null;
> Locale locale = null;
>
> if (null == context || null == messageId) {
> return result;
> }
>
> if (null == (localeObject = context.getContext(Locale.class))) {
> locale = Locale.getDefault();
> }
> else {
> if (localeObject instanceof Locale) {
> locale = (Locale) localeObject;
> }
> else {
> locale = Locale.getDefault();
> }
> }
> if (null != locale) {
> Map threadMap = getCurrentInstance();
> ResourceBundle rb = null;
> if (null == (rb = (ResourceBundle)
> threadMap.get(locale.toString()))) {
> rb = ResourceBundle.getBundle("javax.el.Messages", locale,
> getCurrentLoader(locale));
> threadMap.put(locale.toString(), rb);
> }
> if (null != rb) {
> try {
> result = rb.getString(messageId);
> if (null != params) {
> result = MessageFormat.format(result, params);
> }
> } catch (IllegalArgumentException iae) {
> result = "Can't get localized message: parameters to message appear to be incorrect. Message to format: " + messageId;
> } catch (MissingResourceException mre) {
> result = "Missing Resource in EL implementation: ???" + messageId + "???";
> } catch (Exception e) {
> result = "Exception resolving message in EL implementation: ???" + messageId + "???";
> }
> }
> }
>
> return result;
> }
>
>
>}
>8<---------------------------- Messages.properties
>setPropertyFailed=Can''t set property {0} of type {1} on class {2} to value {3}.
>8<----------------------------
>
>
>
>
>
>
>