dev@javaserverfaces.java.net

Re: [REVIEW] Alignment of ManagedBeanELResolver with spec

From: Jacob Hookom <jacob_at_hookom.net>
Date: Wed, 15 Jun 2005 19:59:41 -0500

Comments below...

Jayashri Visvanathan wrote:

> Hi Jacob,
> jacob_at_hookom.net wrote:
>
>> Also, the ELResolver implementation does 4 HashMap checks because the
>> ApplicationAssociate doesn't provide access to ManagedBean data. It
>> would probably be more efficient if the ApplicationAssociate was able
>> to provide knowledge of a ManagedBean's existence without requiring
>> redundant scope checks on each invocation.
>>
>>
> I think we still need the HashMap checks because according to the
> spec, managed bean may have a property that
> points at an object stored in a scope with a (potentially) shorter
> life span that could have been set programmatically.


That's a good point and started me thinking about a larger issue with
duplicate variable name use....

For example, I could specify a SpringELResolver that already has a
variable of the same name that would be executed after
ManagedBeanELResolver which only checked Request/Session/Application Scope.

Again, I think the problem has the potential of being much larger.
Correcting it would involve changing the order of ELResolvers in the
chain specified in JSP2.8. I would suggest pushing the the
ScopedAttributeELResolver much higher in the chain in order to prevent
redundancy checking in variable contexts that are added to the
application. Granted, the ScopedAttributeELResolver would need to
change its implementation slightly as to not always resolve where base
== null, allowing other variable contexts to resolve later.

Another reason to push the ScopedAttributeELResolver up in the chain is
because it represents the scope that the application programmer uses to
pass information around. Slapping it at the end leaves a lot of room
for gotchas from other ELResolvers and forces them to accomodate as
ManagedBeanELResolver is doing.

If you think it's a possible issue too, I would like to move this over
to the EG.

-- Jacob

>
> Other changes you suggested sound good to me.
> Thanks
> -Jayashri
>
>> -- Jacob
>>
>> Ryan Lubke <Ryan.Lubke_at_Sun.COM> wrote on 06/15/2005, 04:07:47 PM:
>>
>>
>>> SECTION: Modified Files
>>> ----------------------------
>>> M src/com/sun/faces/el/ManagedBeanELResolver.java
>>> - getType() returns null unless base and property are both null
>>> - removed scope searching code from setValue since this should
>>> be deferred to the ScopedAttributeELResolver
>>> - isReadOnly should not call ELContext.setPropertyResolved(true)
>>>
>>> SECTION: Diffs
>>> ----------------------------
>>> Index: src/com/sun/faces/el/ManagedBeanELResolver.java
>>> ===================================================================
>>> RCS file:
>>> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/el/ManagedBeanELResolver.java,v
>>>
>>> retrieving revision 1.3
>>> diff -u -r1.3 ManagedBeanELResolver.java
>>> --- src/com/sun/faces/el/ManagedBeanELResolver.java 10 Jun 2005
>>> 21:59:55 -0000 1.3
>>> +++ src/com/sun/faces/el/ManagedBeanELResolver.java 15 Jun 2005
>>> 13:49:01 -0000
>>> @@ -8,9 +8,10 @@
>>>
>>> package com.sun.faces.el;
>>>
>>> -import com.sun.faces.application.ApplicationAssociate;
>>> -import com.sun.faces.config.ManagedBeanFactory;
>>> -import com.sun.faces.util.Util;
>>> +import java.beans.FeatureDescriptor;
>>> +import java.util.ArrayList;
>>> +import java.util.Iterator;
>>> +import java.util.Map;
>>>
>>> import javax.el.ELContext;
>>> import javax.el.ELException;
>>> @@ -18,11 +19,10 @@
>>> import javax.el.PropertyNotFoundException;
>>> import javax.faces.context.ExternalContext;
>>> import javax.faces.context.FacesContext;
>>> -import java.beans.BeanInfo;
>>> -import java.beans.FeatureDescriptor;
>>> -import java.util.ArrayList;
>>> -import java.util.Iterator;
>>> -import java.util.Map;
>>> +
>>> +import com.sun.faces.application.ApplicationAssociate;
>>> +import com.sun.faces.config.ManagedBeanFactory;
>>> +import com.sun.faces.util.Util;
>>>
>>> public class ManagedBeanELResolver extends ELResolver {
>>>
>>> @@ -34,7 +34,7 @@
>>> if (base != null) {
>>> return null;
>>> }
>>> - if ( base == null && property == null) {
>>> + if (property == null) {
>>> String message = Util.getExceptionMessageString
>>> (Util.NULL_PARAMETERS_ERROR_MESSAGE_ID);
>>> message = message + " base " + base + " property " +
>>> property;
>>> @@ -44,22 +44,13 @@
>>> Object result = null;
>>> FacesContext facesContext = (FacesContext)
>>> context.getContext(FacesContext.class);
>>> - ExternalContext externalContext =
>>> facesContext.getExternalContext();
>>> - boolean beanExists = false;
>>> - - if (externalContext.getRequestMap().containsKey(property)) {
>>> - beanExists = true;
>>> - }
>>> - else if (externalContext.getSessionMap().containsKey(property)) {
>>> - beanExists = true;
>>> - }
>>> - else if
>>> (externalContext.getApplicationMap().containsKey(property)) {
>>> - beanExists = true;
>>> - }
>>> -
>>> - if (beanExists) {
>>> - return null;
>>> - }
>>> + ExternalContext externalContext =
>>> facesContext.getExternalContext();
>>> +
>>> + if (externalContext.getRequestMap().containsKey(property)
>>> + || externalContext.getSessionMap().containsKey(property)
>>> + ||
>>> externalContext.getApplicationMap().containsKey(property)) {
>>> + return null;
>>> + }
>>> // if it's a managed bean, try to create it
>>> ApplicationAssociate associate = ApplicationAssociate
>>> @@ -77,50 +68,28 @@
>>>
>>> public Class getType(ELContext context, Object base, Object
>>> property)
>>> throws ELException {
>>> - if (base != null) {
>>> - return null;
>>> - }
>>> - if ( base == null && property == null) {
>>> +
>>> + if (base == null && property == null) {
>>> String message = Util.getExceptionMessageString
>>> (Util.NULL_PARAMETERS_ERROR_MESSAGE_ID);
>>> message = message + " base " + base + " property " +
>>> property;
>>> throw new PropertyNotFoundException(message);
>>> }
>>> - context.setPropertyResolved(true);
>>> - return Object.class;
>>> +
>>> + return null;
>>> +
>>> }
>>>
>>> public void setValue(ELContext context, Object base, Object
>>> property,
>>> Object val) throws ELException {
>>> - - if (base != null) {
>>> - return;
>>> - }
>>> - if ( base == null && property == null) {
>>> +
>>> + if (base == null && property == null) {
>>> String message = Util.getExceptionMessageString
>>> (Util.NULL_PARAMETERS_ERROR_MESSAGE_ID);
>>> message = message + " base " + base + " property " +
>>> property;
>>> throw new PropertyNotFoundException(message);
>>> }
>>> - - Object result = null;
>>> - FacesContext facesContext =
>>> - (FacesContext) context.getContext(FacesContext.class);
>>> - String attribute = (String) property;
>>> - ExternalContext ec = facesContext.getExternalContext();
>>> - if ((result = ec.getRequestMap().get(attribute)) != null) {
>>> - ec.getRequestMap().put(attribute, val);
>>> - context.setPropertyResolved(true);
>>> - } else if ((result = ec.getSessionMap().get(attribute)) !=
>>> null) {
>>> - context.setPropertyResolved(true);
>>> - ec.getSessionMap().put(attribute, val);
>>> - } else if ((result = ec.getApplicationMap().get(attribute))
>>> != null) {
>>> - context.setPropertyResolved(true);
>>> - ec.getApplicationMap().put(attribute, val);
>>> - } else {
>>> - ec.getRequestMap().put(attribute, val);
>>> - }
>>> - +
>>> }
>>>
>>> public boolean isReadOnly(ELContext context, Object base, Object
>>> property)
>>> @@ -128,15 +97,13 @@
>>> if (base != null) {
>>> return false;
>>> }
>>> - if ( base == null && property == null) {
>>> + if (property == null) {
>>> String message = Util.getExceptionMessageString
>>> (Util.NULL_PARAMETERS_ERROR_MESSAGE_ID);
>>> message = message + " base " + base + " property " +
>>> property;
>>> throw new PropertyNotFoundException(message);
>>> }
>>> - // return value will be ignored unless
>>> context.propertyResolved is
>>> - // set to true.
>>> - context.setPropertyResolved(true);
>>> +
>>> return false;
>>> }
>>>
>>> @@ -147,7 +114,6 @@
>>> }
>>>
>>> ArrayList list = new ArrayList();
>>> - BeanInfo info = null;
>>> FacesContext facesContext =
>>> (FacesContext) context.getContext(FacesContext.class);
>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
>>> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
>> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>>
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>
>


-- 
Jacob Hookom - Minneapolis
--------------------------
http://hookom.blogspot.com