Re: svn commit: r530154 - /myfaces/core/branches/jsf12/impl/src/main/java/org/apache/myfaces/el/unified/resolver/ManagedBeanResolver.java

2007-04-23 Thread Paul McMahan 

On Apr 19, 2007, at 10:25 AM, Martin Marinschek wrote:


oh, yes. I had overlooked the return null at the end of the method -
that was different before.


Brief update here - I ended up changing the behavior of  
ManagedBeanResolver to go ahead and return the managed bean right  
away due to :

https://issues.apache.org/jira/browse/MYFACES-1593

Best wishes,
Paul

Re: svn commit: r530154 - /myfaces/core/branches/jsf12/impl/src/main/java/org/apache/myfaces/el/unified/resolver/ManagedBeanResolver.java

2007-04-20 Thread Martin Marinschek 

Sure!

regards,

Martin

On 4/19/07, Paul McMahan [EMAIL PROTECTED] wrote:

Cycle reference check should be fixed now in r530517.  thanks again for the
peer review.

Best wishes,
Paul


On Apr 19, 2007, at 10:25 AM, Martin Marinschek wrote:


But still - you are short-circuiting the cyclic reference check now -

if I have a bean now which has scope none, and has a managed-property

referring to the bean again, then we'll run into an infinite loop,

right?




--

http://www.irian.at

Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German

Professional Support for Apache MyFaces

Re: svn commit: r530154 - /myfaces/core/branches/jsf12/impl/src/main/java/org/apache/myfaces/el/unified/resolver/ManagedBeanResolver.java

2007-04-19 Thread Paul McMahan 
Thanks for the review Martin.  I agree that the persistence mechanism  
for managed beans was already working OK.   i.e. when a managed bean  
has scope none it was not persisted in a scope.  But the problem I  
encountered was that the CompositeELResolver was not able to resolve  
managed beans with scope none.Managed beans with scope  
request, session, or application could be resolved because  
after ManagedBeanResolver creates the bean it persists it in a scope,  
returns null, and then relies on ScopedAttributeResolver to resolve  
it further down the call stack.  But when scope is none persisting  
the bean in scope was a no-op, so the exchange between  
ManagedBeanResolver and ScopedAttributeResolver didn't end up  
resolving the bean for CompositeELResolver.


The second change allows ManagedBeanResolver.getValue() to go ahead  
and return the managed bean immediately instead of returning null.   
The first change (removing none from s_standardScopes) was really  
just clean up since that code won't be used.


Best wishes,
Paul

On Apr 18, 2007, at 5:13 PM, Martin Marinschek wrote:


Hi Paul,

if you do the first change (introduce a scope where put does nothing),
I don't see why the second one needs to be done - putting will do
nothing, so you don't need the extra-check for none, right?

regards,

Martin

On 4/18/07, Paul McMahan [EMAIL PROTECTED] wrote:

Just wanted to invite some peer review for this change I just
committed for MYFACES-1588.  The problem was that managed beans in
scope none weren't accessible via the resolver.  The change I made
passes the test cases but there might be a more elegant way to
implement it.

Also, I have an update for the ValueBindingImplCactus.java test case
to check for this bug (looked like a good place for it) but I
couldn't figure out how to run cactus from maven.  Does that work OK
and if so can anyone provide tips on how to execute?

Best wishes,
Paul

On Apr 18, 2007, at 4:53 PM, [EMAIL PROTECTED] wrote:

 Author: pmcmahan
 Date: Wed Apr 18 13:53:26 2007
 New Revision: 530154

 URL: http://svn.apache.org/viewvc?view=revrev=530154
 Log:
 MYFACES-1588 resolve managed beans in scope none

 Modified:
 myfaces/core/branches/jsf12/impl/src/main/java/org/apache/
 myfaces/el/unified/resolver/ManagedBeanResolver.java

 Modified: myfaces/core/branches/jsf12/impl/src/main/java/org/ 
apache/

 myfaces/el/unified/resolver/ManagedBeanResolver.java
 URL: http://svn.apache.org/viewvc/myfaces/core/branches/jsf12/impl/
 src/main/java/org/apache/myfaces/el/unified/resolver/
 ManagedBeanResolver.java?view=diffrev=530154r1=530153r2=530154
  
= 
=

 
 --- myfaces/core/branches/jsf12/impl/src/main/java/org/apache/
 myfaces/el/unified/resolver/ManagedBeanResolver.java (original)
 +++ myfaces/core/branches/jsf12/impl/src/main/java/org/apache/
 myfaces/el/unified/resolver/ManagedBeanResolver.java Wed Apr 18
 13:53:26 2007
 @@ -74,15 +74,6 @@
  extContext.getApplicationMap().put(name, obj);
  }
  });
 -s_standardScopes.put(
 -none,
 -new Scope()
 -{
 -public void put(ExternalContext extContext, String
 name, Object obj)
 -{
 -// do nothing
 -}
 -});
  }

  /**
 @@ -156,8 +147,13 @@

  ManagedBean managedBean = runtimeConfig
 (context).getManagedBean(strProperty);
  if (managedBean != null) {
 -storeManagedBean(managedBean, facesContext(context));
 +FacesContext facesContext = facesContext(context);
  context.setPropertyResolved(true);
 +if (none.equals(managedBean.getManagedBeanScope 
())) {

 +return beanBuilder.buildManagedBean(facesContext,
 managedBean);
 +} else {
 +storeManagedBean(managedBean, facesContext);
 +}
  }

  return null;







--

http://www.irian.at

Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German

Professional Support for Apache MyFaces

Re: svn commit: r530154 - /myfaces/core/branches/jsf12/impl/src/main/java/org/apache/myfaces/el/unified/resolver/ManagedBeanResolver.java

2007-04-19 Thread Martin Marinschek 

Hi Paul,

oh, yes. I had overlooked the return null at the end of the method -
that was different before.

But still - you are short-circuiting the cyclic reference check now -
if I have a bean now which has scope none, and has a managed-property
referring to the bean again, then we'll run into an infinite loop,
right?

regards,

Martin

On 4/19/07, Paul McMahan [EMAIL PROTECTED] wrote:

Thanks for the review Martin.  I agree that the persistence mechanism
for managed beans was already working OK.   i.e. when a managed bean
has scope none it was not persisted in a scope.  But the problem I
encountered was that the CompositeELResolver was not able to resolve
managed beans with scope none.Managed beans with scope
request, session, or application could be resolved because
after ManagedBeanResolver creates the bean it persists it in a scope,
returns null, and then relies on ScopedAttributeResolver to resolve
it further down the call stack.  But when scope is none persisting
the bean in scope was a no-op, so the exchange between
ManagedBeanResolver and ScopedAttributeResolver didn't end up
resolving the bean for CompositeELResolver.

The second change allows ManagedBeanResolver.getValue() to go ahead
and return the managed bean immediately instead of returning null.
The first change (removing none from s_standardScopes) was really
just clean up since that code won't be used.

Best wishes,
Paul

On Apr 18, 2007, at 5:13 PM, Martin Marinschek wrote:

 Hi Paul,

 if you do the first change (introduce a scope where put does nothing),
 I don't see why the second one needs to be done - putting will do
 nothing, so you don't need the extra-check for none, right?

 regards,

 Martin

 On 4/18/07, Paul McMahan [EMAIL PROTECTED] wrote:
 Just wanted to invite some peer review for this change I just
 committed for MYFACES-1588.  The problem was that managed beans in
 scope none weren't accessible via the resolver.  The change I made
 passes the test cases but there might be a more elegant way to
 implement it.

 Also, I have an update for the ValueBindingImplCactus.java test case
 to check for this bug (looked like a good place for it) but I
 couldn't figure out how to run cactus from maven.  Does that work OK
 and if so can anyone provide tips on how to execute?

 Best wishes,
 Paul

 On Apr 18, 2007, at 4:53 PM, [EMAIL PROTECTED] wrote:

  Author: pmcmahan
  Date: Wed Apr 18 13:53:26 2007
  New Revision: 530154
 
  URL: http://svn.apache.org/viewvc?view=revrev=530154
  Log:
  MYFACES-1588 resolve managed beans in scope none
 
  Modified:
  myfaces/core/branches/jsf12/impl/src/main/java/org/apache/
  myfaces/el/unified/resolver/ManagedBeanResolver.java
 
  Modified: myfaces/core/branches/jsf12/impl/src/main/java/org/
 apache/
  myfaces/el/unified/resolver/ManagedBeanResolver.java
  URL: http://svn.apache.org/viewvc/myfaces/core/branches/jsf12/impl/
  src/main/java/org/apache/myfaces/el/unified/resolver/
  ManagedBeanResolver.java?view=diffrev=530154r1=530153r2=530154
 
 =
 =
  
  --- myfaces/core/branches/jsf12/impl/src/main/java/org/apache/
  myfaces/el/unified/resolver/ManagedBeanResolver.java (original)
  +++ myfaces/core/branches/jsf12/impl/src/main/java/org/apache/
  myfaces/el/unified/resolver/ManagedBeanResolver.java Wed Apr 18
  13:53:26 2007
  @@ -74,15 +74,6 @@
   extContext.getApplicationMap().put(name, obj);
   }
   });
  -s_standardScopes.put(
  -none,
  -new Scope()
  -{
  -public void put(ExternalContext extContext, String
  name, Object obj)
  -{
  -// do nothing
  -}
  -});
   }
 
   /**
  @@ -156,8 +147,13 @@
 
   ManagedBean managedBean = runtimeConfig
  (context).getManagedBean(strProperty);
   if (managedBean != null) {
  -storeManagedBean(managedBean, facesContext(context));
  +FacesContext facesContext = facesContext(context);
   context.setPropertyResolved(true);
  +if (none.equals(managedBean.getManagedBeanScope
 ())) {
  +return beanBuilder.buildManagedBean(facesContext,
  managedBean);
  +} else {
  +storeManagedBean(managedBean, facesContext);
  +}
   }
 
   return null;
 
 




 --

 http://www.irian.at

 Your JSF powerhouse -
 JSF Consulting, Development and
 Courses in English and German

 Professional Support for Apache MyFaces






--

http://www.irian.at

Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German

Professional Support for Apache MyFaces

Re: svn commit: r530154 - /myfaces/core/branches/jsf12/impl/src/main/java/org/apache/myfaces/el/unified/resolver/ManagedBeanResolver.java

2007-04-19 Thread Paul McMahan 

On Apr 19, 2007, at 10:25 AM, Martin Marinschek wrote:


But still - you are short-circuiting the cyclic reference check now -
if I have a bean now which has scope none, and has a managed-property
referring to the bean again, then we'll run into an infinite loop,
right?


Yes that's true, and thanks for pointing it out.  I'll need to stare  
at this a bit longer to figure out a better solution.  Suggestions  
are welcome.


Best wishes,
Paul

Re: svn commit: r530154 - /myfaces/core/branches/jsf12/impl/src/main/java/org/apache/myfaces/el/unified/resolver/ManagedBeanResolver.java

2007-04-19 Thread Paul McMahan 
Cycle reference check should be fixed now in r530517.  thanks again  
for the peer review.


Best wishes,
Paul

On Apr 19, 2007, at 10:25 AM, Martin Marinschek wrote:


But still - you are short-circuiting the cyclic reference check now -
if I have a bean now which has scope none, and has a managed-property
referring to the bean again, then we'll run into an infinite loop,
right?

Re: svn commit: r530154 - /myfaces/core/branches/jsf12/impl/src/main/java/org/apache/myfaces/el/unified/resolver/ManagedBeanResolver.java

2007-04-18 Thread Paul McMahan 
Just wanted to invite some peer review for this change I just  
committed for MYFACES-1588.  The problem was that managed beans in  
scope none weren't accessible via the resolver.  The change I made  
passes the test cases but there might be a more elegant way to  
implement it.


Also, I have an update for the ValueBindingImplCactus.java test case  
to check for this bug (looked like a good place for it) but I  
couldn't figure out how to run cactus from maven.  Does that work OK  
and if so can anyone provide tips on how to execute?


Best wishes,
Paul

On Apr 18, 2007, at 4:53 PM, [EMAIL PROTECTED] wrote:


Author: pmcmahan
Date: Wed Apr 18 13:53:26 2007
New Revision: 530154

URL: http://svn.apache.org/viewvc?view=revrev=530154
Log:
MYFACES-1588 resolve managed beans in scope none

Modified:
myfaces/core/branches/jsf12/impl/src/main/java/org/apache/ 
myfaces/el/unified/resolver/ManagedBeanResolver.java


Modified: myfaces/core/branches/jsf12/impl/src/main/java/org/apache/ 
myfaces/el/unified/resolver/ManagedBeanResolver.java
URL: http://svn.apache.org/viewvc/myfaces/core/branches/jsf12/impl/ 
src/main/java/org/apache/myfaces/el/unified/resolver/ 
ManagedBeanResolver.java?view=diffrev=530154r1=530153r2=530154
== 

--- myfaces/core/branches/jsf12/impl/src/main/java/org/apache/ 
myfaces/el/unified/resolver/ManagedBeanResolver.java (original)
+++ myfaces/core/branches/jsf12/impl/src/main/java/org/apache/ 
myfaces/el/unified/resolver/ManagedBeanResolver.java Wed Apr 18  
13:53:26 2007

@@ -74,15 +74,6 @@
 extContext.getApplicationMap().put(name, obj);
 }
 });
-s_standardScopes.put(
-none,
-new Scope()
-{
-public void put(ExternalContext extContext, String  
name, Object obj)

-{
-// do nothing
-}
-});
 }

 /**
@@ -156,8 +147,13 @@

 ManagedBean managedBean = runtimeConfig 
(context).getManagedBean(strProperty);

 if (managedBean != null) {
-storeManagedBean(managedBean, facesContext(context));
+FacesContext facesContext = facesContext(context);
 context.setPropertyResolved(true);
+if (none.equals(managedBean.getManagedBeanScope())) {
+return beanBuilder.buildManagedBean(facesContext,  
managedBean);

+} else {
+storeManagedBean(managedBean, facesContext);
+}
 }

 return null;

Re: svn commit: r530154 - /myfaces/core/branches/jsf12/impl/src/main/java/org/apache/myfaces/el/unified/resolver/ManagedBeanResolver.java

2007-04-18 Thread Dennis Byrne 

I don't think anyone has run the cactus tests in about six months.  They
aren't a part of the CI loop either.

Dennis Byrne

On 4/18/07, Paul McMahan [EMAIL PROTECTED] wrote:


Just wanted to invite some peer review for this change I just
committed for MYFACES-1588.  The problem was that managed beans in
scope none weren't accessible via the resolver.  The change I made
passes the test cases but there might be a more elegant way to
implement it.

Also, I have an update for the ValueBindingImplCactus.java test case
to check for this bug (looked like a good place for it) but I
couldn't figure out how to run cactus from maven.  Does that work OK
and if so can anyone provide tips on how to execute?

Best wishes,
Paul

On Apr 18, 2007, at 4:53 PM, [EMAIL PROTECTED] wrote:

 Author: pmcmahan
 Date: Wed Apr 18 13:53:26 2007
 New Revision: 530154

 URL: http://svn.apache.org/viewvc?view=revrev=530154
 Log:
 MYFACES-1588 resolve managed beans in scope none

 Modified:
 myfaces/core/branches/jsf12/impl/src/main/java/org/apache/
 myfaces/el/unified/resolver/ManagedBeanResolver.java

 Modified: myfaces/core/branches/jsf12/impl/src/main/java/org/apache/
 myfaces/el/unified/resolver/ManagedBeanResolver.java
 URL: http://svn.apache.org/viewvc/myfaces/core/branches/jsf12/impl/
 src/main/java/org/apache/myfaces/el/unified/resolver/
 ManagedBeanResolver.java?view=diffrev=530154r1=530153r2=530154
 ==
 
 --- myfaces/core/branches/jsf12/impl/src/main/java/org/apache/
 myfaces/el/unified/resolver/ManagedBeanResolver.java (original)
 +++ myfaces/core/branches/jsf12/impl/src/main/java/org/apache/
 myfaces/el/unified/resolver/ManagedBeanResolver.java Wed Apr 18
 13:53:26 2007
 @@ -74,15 +74,6 @@
  extContext.getApplicationMap().put(name, obj);
  }
  });
 -s_standardScopes.put(
 -none,
 -new Scope()
 -{
 -public void put(ExternalContext extContext, String
 name, Object obj)
 -{
 -// do nothing
 -}
 -});
  }

  /**
 @@ -156,8 +147,13 @@

  ManagedBean managedBean = runtimeConfig
 (context).getManagedBean(strProperty);
  if (managedBean != null) {
 -storeManagedBean(managedBean, facesContext(context));
 +FacesContext facesContext = facesContext(context);
  context.setPropertyResolved(true);
 +if (none.equals(managedBean.getManagedBeanScope())) {
 +return beanBuilder.buildManagedBean(facesContext,
 managedBean);
 +} else {
 +storeManagedBean(managedBean, facesContext);
 +}
  }

  return null;







--
Dennis Byrne

Re: svn commit: r530154 - /myfaces/core/branches/jsf12/impl/src/main/java/org/apache/myfaces/el/unified/resolver/ManagedBeanResolver.java

2007-04-18 Thread Matthias Wessendorf 

right,

I think they used to be Bill's sandbox ;)

-M

On 4/18/07, Dennis Byrne [EMAIL PROTECTED] wrote:

I don't think anyone has run the cactus tests in about six months.  They
aren't a part of the CI loop either.

Dennis Byrne


On 4/18/07, Paul McMahan [EMAIL PROTECTED] wrote:
 Just wanted to invite some peer review for this change I just
 committed for MYFACES-1588.  The problem was that managed beans in
 scope none weren't accessible via the resolver.  The change I made
 passes the test cases but there might be a more elegant way to
 implement it.

 Also, I have an update for the ValueBindingImplCactus.java test case
 to check for this bug (looked like a good place for it) but I
 couldn't figure out how to run cactus from maven.  Does that work OK
 and if so can anyone provide tips on how to execute?

 Best wishes,
 Paul

 On Apr 18, 2007, at 4:53 PM, [EMAIL PROTECTED] wrote:

  Author: pmcmahan
  Date: Wed Apr 18 13:53:26 2007
  New Revision: 530154
 
  URL: http://svn.apache.org/viewvc?view=revrev=530154
  Log:
  MYFACES-1588 resolve managed beans in scope none
 
  Modified:
 
myfaces/core/branches/jsf12/impl/src/main/java/org/apache/
  myfaces/el/unified/resolver/ManagedBeanResolver.java
 
  Modified:
myfaces/core/branches/jsf12/impl/src/main/java/org/apache/
  myfaces/el/unified/resolver/ManagedBeanResolver.java
  URL:
http://svn.apache.org/viewvc/myfaces/core/branches/jsf12/impl/
  src/main/java/org/apache/myfaces/el/unified/resolver/
 
ManagedBeanResolver.java?view=diffrev=530154r1=530153r2=530154
 
==
  
  ---
myfaces/core/branches/jsf12/impl/src/main/java/org/apache/
  myfaces/el/unified/resolver/ManagedBeanResolver.java
(original)
  +++
myfaces/core/branches/jsf12/impl/src/main/java/org/apache/
  myfaces/el/unified/resolver/ManagedBeanResolver.java
Wed Apr 18
  13:53:26 2007
  @@ -74,15 +74,6 @@
   extContext.getApplicationMap().put(name, obj);
   }
   });
  -s_standardScopes.put(
  -none,
  -new Scope()
  -{
  -public void put(ExternalContext extContext, String
  name, Object obj)
  -{
  -// do nothing
  -}
  -});
   }
 
   /**
  @@ -156,8 +147,13 @@
 
   ManagedBean managedBean = runtimeConfig
  (context).getManagedBean(strProperty);
   if (managedBean != null) {
  -storeManagedBean(managedBean,
facesContext(context));
  +FacesContext facesContext = facesContext(context);
   context.setPropertyResolved(true);
  +if (none.equals(managedBean.getManagedBeanScope())) {
  +return beanBuilder.buildManagedBean(facesContext,
  managedBean);
  +} else {
  +storeManagedBean(managedBean,
facesContext);
  +}
   }
 
   return null;
 
 





--
Dennis Byrne



--
Matthias Wessendorf
http://tinyurl.com/fmywh

further stuff:
blog: http://jroller.com/page/mwessendorf
mail: mwessendorf-at-gmail-dot-com

Re: svn commit: r530154 - /myfaces/core/branches/jsf12/impl/src/main/java/org/apache/myfaces/el/unified/resolver/ManagedBeanResolver.java

2007-04-18 Thread Martin Marinschek 

Hi Paul,

if you do the first change (introduce a scope where put does nothing),
I don't see why the second one needs to be done - putting will do
nothing, so you don't need the extra-check for none, right?

regards,

Martin

On 4/18/07, Paul McMahan [EMAIL PROTECTED] wrote:

Just wanted to invite some peer review for this change I just
committed for MYFACES-1588.  The problem was that managed beans in
scope none weren't accessible via the resolver.  The change I made
passes the test cases but there might be a more elegant way to
implement it.

Also, I have an update for the ValueBindingImplCactus.java test case
to check for this bug (looked like a good place for it) but I
couldn't figure out how to run cactus from maven.  Does that work OK
and if so can anyone provide tips on how to execute?

Best wishes,
Paul

On Apr 18, 2007, at 4:53 PM, [EMAIL PROTECTED] wrote:

 Author: pmcmahan
 Date: Wed Apr 18 13:53:26 2007
 New Revision: 530154

 URL: http://svn.apache.org/viewvc?view=revrev=530154
 Log:
 MYFACES-1588 resolve managed beans in scope none

 Modified:
 myfaces/core/branches/jsf12/impl/src/main/java/org/apache/
 myfaces/el/unified/resolver/ManagedBeanResolver.java

 Modified: myfaces/core/branches/jsf12/impl/src/main/java/org/apache/
 myfaces/el/unified/resolver/ManagedBeanResolver.java
 URL: http://svn.apache.org/viewvc/myfaces/core/branches/jsf12/impl/
 src/main/java/org/apache/myfaces/el/unified/resolver/
 ManagedBeanResolver.java?view=diffrev=530154r1=530153r2=530154
 ==
 
 --- myfaces/core/branches/jsf12/impl/src/main/java/org/apache/
 myfaces/el/unified/resolver/ManagedBeanResolver.java (original)
 +++ myfaces/core/branches/jsf12/impl/src/main/java/org/apache/
 myfaces/el/unified/resolver/ManagedBeanResolver.java Wed Apr 18
 13:53:26 2007
 @@ -74,15 +74,6 @@
  extContext.getApplicationMap().put(name, obj);
  }
  });
 -s_standardScopes.put(
 -none,
 -new Scope()
 -{
 -public void put(ExternalContext extContext, String
 name, Object obj)
 -{
 -// do nothing
 -}
 -});
  }

  /**
 @@ -156,8 +147,13 @@

  ManagedBean managedBean = runtimeConfig
 (context).getManagedBean(strProperty);
  if (managedBean != null) {
 -storeManagedBean(managedBean, facesContext(context));
 +FacesContext facesContext = facesContext(context);
  context.setPropertyResolved(true);
 +if (none.equals(managedBean.getManagedBeanScope())) {
 +return beanBuilder.buildManagedBean(facesContext,
 managedBean);
 +} else {
 +storeManagedBean(managedBean, facesContext);
 +}
  }

  return null;







--

http://www.irian.at

Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German

Professional Support for Apache MyFaces