You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org> on 2010/03/23 16:45:27 UTC

[jira] Created: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

BeanValidation does not work with Unified EL 2.2
------------------------------------------------

                 Key: MYFACES-2621
                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
             Project: MyFaces Core
          Issue Type: Bug
          Components: JSR-314
    Affects Versions: 2.0.0-beta-3
         Environment: bean validation 1.0.0.GA, unified el 2.2
            Reporter: Jakob Korherr
            Assignee: Jakob Korherr


When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.

See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12848916#action_12848916 ] 

Leonardo Uribe commented on MYFACES-2621:
-----------------------------------------

I have this list of wrappers on ValueExpression:

javax.faces.component._ValueBindingToValueExpression (not used anymore, and in this case not relevant)
org.apache.myfaces.view.facelets.tag.jstl.core.IndexedValueExpression
org.apache.myfaces.view.facelets.el.TagValueExpression
org.apache.myfaces.view.facelets.el.LocationValueExpression
org.apache.myfaces.view.facelets.el.ELText.LiteralValueExpression


> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Jan-Kees van Andel (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12851358#action_12851358 ] 

Jan-Kees van Andel commented on MYFACES-2621:
---------------------------------------------

Looks good, you only need to declare the fields volatile, to ensure safe publication of the values.

Performance impact of a volatile read is no issue.

> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>             Fix For: 2.0.0-beta-4
>
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12848829#action_12848829 ] 

Jakob Korherr commented on MYFACES-2621:
----------------------------------------

The problem here is that the ValueExpression on which we invoke getValueReference() via reflection is a TagValueExpression and not directly a ValueExpression implementation of the unified el 2.2. So getValueReference() always returns null here, because it is not implemented.

We now have two opportunities:

1) Just don't care if el-2.2 is available and get the right base and property the old fashioned way
2) Try to extract the real ValueExpression out of a series of wrappers and invoke getValueReference() on it

I tend to 1) here, because a) it will always work correctly and b) we won't be much slower as with 2) here, because we need to use reflection here a couple of times in order to make it work and I don't think that this is very fast.

Anyway, I will do some performance tests and find out what is faster.

Does anyone know if there is a scenario where our "old-fashioned" way to retrieve the base and the property of the ValueExpression will break with the new unified el?

> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12848899#action_12848899 ] 

Leonardo Uribe commented on MYFACES-2621:
-----------------------------------------

Use reflection has a performance hit. If we use duplicate wrappers, the only problem is the synchronized methods used for check if EL is available, but since this code is called many times, use those methods are not a good idea. We have to check which solution has better performance.

Other idea we could use is make TagValueExpression implements FacesWrapper, so we can get the inner ValueExpression and call it directly.

If there exists other ValueExpression wrappers that need to be handled, we need a list of the ones required to change in myfaces.

The signature of ValueExpression.getValueReference is this:

 ValueReference 	getValueReference(ELContext context) 

My question is: Does really match the method with the proposed signature?

 Object getValueReference(ELContext context) 

If so, maybe use reflection is the way to go, but we have to try.

> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12851399#action_12851399 ] 

Jakob Korherr commented on MYFACES-2621:
----------------------------------------

OK thanks, Jan-Kees. I just added volatile to all fields.

> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>             Fix For: 2.0.0-beta-4
>
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12849238#action_12849238 ] 

Jakob Korherr commented on MYFACES-2621:
----------------------------------------

We need the ValueReference because it contains the base object and the property name to which the ValueExpression resolves. This is needed in BeanValidator, because we need to get the class of the managed bean and the right property name to get the right annotations out of it!

Now with el-api 2.1 there is the trick to resolve the ValueExpression with a decorated ELContext and a special ELResolver to get both base object and property name (see class BeanValidator for the exact implementation). The new el-api however provides a method for this case called getValueReference() and the spec says that if it is available, we must use it instead of the trick.

I'll try some things here, reflection and/or custom wrappers and figure out what is the best!

> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12850700#action_12850700 ] 

Jakob Korherr commented on MYFACES-2621:
----------------------------------------

Thinking more about the problem with the wrappers I came up with the following solution: just create a sub-class of every wrapper that overrides ValueReference correctly and choose at creation time whether to use the "original" wrapper or the sub-class. I also already tried it out with TagValueExpression and it works just fine!

Simple, easy, fast. Any objections to this solution?

> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.
     [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jakob Korherr resolved MYFACES-2621.
------------------------------------

       Resolution: Fixed
    Fix Version/s: 2.0.0-beta-4

I removed synchronized from all ExternalSpecifications methods, because this is not needed. This issue is now fixed!

> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>             Fix For: 2.0.0-beta-4
>
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12848851#action_12848851 ] 

Jakob Korherr commented on MYFACES-2621:
----------------------------------------

We could do something like this in all our ValueExpression wrapper classes:

public Object getValueReference(ELContext context)
    {
        try
        {
            Method method = ValueExpression.class.getMethod("getValueReference", ELContext.class);
            return method.invoke(this.orig, context);
        }
        catch (Exception e)
        {
            return null;
        }
    }

I just tested it and it works great. What do you think?

> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Martin Marinschek (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12850902#action_12850902 ] 

Martin Marinschek commented on MYFACES-2621:
--------------------------------------------

Sounds like a totally sure case for removing synchronized ;)

best regards,

Martin

> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12849379#action_12849379 ] 

Jakob Korherr commented on MYFACES-2621:
----------------------------------------

I committed a first solution to this problem. The code tries to use getValueReference() if el-api 2.2 is available and falls back to the "normal" way if this returns null.

This works for all scenarios, except that getValueReference() always returns null for our wrapped ValueExpressions. I added a todo for that on the code and I will work on that tomorrow!

> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12848884#action_12848884 ] 

Jakob Korherr commented on MYFACES-2621:
----------------------------------------

Totally right!

Of course, we could check if unified el 2.2 is available before the creation of the TagValueExpression and use an el-2.2 aware implementation of TagValueExpression instead. But this would lead to lots of duplicate code, because TagValueExpression is not the only ValueExpression wrapper. There are some more.

Furthermore I don't think that this is that slow when using reflection. Also a big advantage here is for future versions of MyFaces that have el-2.2 as a requirement that we just have to change this one method to not use reflection in every wrapper.

I am prefering the reflection way, but if you think that this is just too slow, we can also have the duplicate wrappers ;)

> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Jan-Kees van Andel (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12848895#action_12848895 ] 

Jan-Kees van Andel commented on MYFACES-2621:
---------------------------------------------

My first implementation of ExternalSpecifications was purely designed for performance, relying on the ClassLoader to safely publish the variables. Testability was the reason to refactor it into synchronized methods.
If you guys don't mind about less testability, we can always revert it to the original implementation which is probably the fastest possible implementation (no volatile/synchronized needed).

A Double Checked Locking (DCL) implementation would also be a valid choice, since we depend on Java 5+ and the Java 5 JMM guarantees DCL correctness. That way, the first access requires an exclusive lock and consecutive calls only do volatile reads. This is still pretty fast without compromising testability.

I would say, revert ExternalSpecifications to its original form and then look at Leonardo's suggestion.

> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12850895#action_12850895 ] 

Jakob Korherr commented on MYFACES-2621:
----------------------------------------

Looking at all the different ValueExpression wrappers I figured out the following:

TagValueExpression - needed for bean validation: created TagValueExpressionUEL
LocationValueExpression - needed for bean validation: created LocationValueExpressionUEL
ValueBindingToValueExpression - not needed: only used in ApplicationImpl to create a component with a binding
MappedValueExpression, IndexedValueExpression, IteratedValueExpression - not needed: only used for c:forEach and bean validation does not work with properties of Map, List, array... because they can't be annotated
LiteralValueExpression - not needed: bean validation only works for "real" properties of a class

So I'll commit the solution with TagValueExpressionUEL and LocationValueExpressionUEL.

> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12848914#action_12848914 ] 

Leonardo Uribe commented on MYFACES-2621:
-----------------------------------------

I think we don't really need synchronize methods on ExternalSpecifications. The reason is the variables:

    private static Boolean beanValidationAvailable;
    private static Boolean unifiedELAvailable;

only changes from null to Boolean.TRUE or from null to Boolean.FALSE, and after they are initialized, it never changes. So is multiple threads set this variable at the same time, everyone will set it with the same value (true or false, but never null). Ok, if we want to prevent that why don't use:

    private static volatile Boolean beanValidationAvailable;
    private static volatile Boolean unifiedELAvailable;

that is "one time safe publication" pattern.

> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12848832#action_12848832 ] 

Jakob Korherr commented on MYFACES-2621:
----------------------------------------

OK, change of plan. The javadoc of BeanValidator really says "If this application is running in an environment with a Unified EL Implementation for Java EE6 or later, obtain the ValueReference from valueExpression...", so we have to find a way to make this work correctly with wrappers.

> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12848866#action_12848866 ] 

Leonardo Uribe commented on MYFACES-2621:
-----------------------------------------

org.apache.myfaces.view.facelets.el.TagValueExpression is created only from org.apache.myfaces.view.facelets.tag.TagAttributeImpl, and it is used as a wrapper from the real ValueExpression, right?

Other alternative could be do something on TagAttributeImpl, so it uses another class like TagValueExpression, but that one override getValueReference, passing the call to the original one. The problem with this one is that this code is called many, many times, so it is critical to have an optimal implementation, and the current implementation of ExternalSpecifications has some synchronized methods that doesn't look good from my point of view. This code was let as is to allow run test configurations easily, but in this case I think it is more important code performace.

> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12848924#action_12848924 ] 

Leonardo Uribe commented on MYFACES-2621:
-----------------------------------------

I miss one wrapper:

org.apache.myfaces.view.facelets.tag.jstl.core.IteratedValueExpression

In the case of LocationValueExpression, IndexedValueExpression and IteratedValueExpression  we can use two wrappers hack. I'm just exploring ideas, to see which solution looks better.

Really I don't like to change the signature of a method. If the method does not compile with el-api 2.2, that says something is wrong. In my personal opinion, the fact that it works, but does not compile with el-api 2.2 makes it not viable, because we could expect in the future (other versions of jsf) to upgrade to el-api 2.2, and in that case our code will break.

Other thing we can do is when ValueReference returns null, do the same that we do when el 2.2 is not available, but I ignore the implications of do this. All depends of why and what we do with ValueReference in bean validation. Note I don't know all details behind current bean validation implementation.


> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12848918#action_12848918 ] 

Jakob Korherr commented on MYFACES-2621:
----------------------------------------

I totally agree with you Leonardo here! Also I want to be able to set those field using reflection for automated tests, so a big -1 from me on reverting ExternalSpecifications!!!!

We can use Object getValueReference(ELContext context) here, because MyFaces is compiled with the el-api 2.1 dependency and so it does not know the newer method from the super class with return value ValueReference. At first I was also not sure if this works, but I tested it and it did. Of course, it won't compile with el-api 2.2, but the only thing to change here would be the return type...

To your suggestion with FacesWrapper, Leonardo: we can't do this, because there are wrappers that have to do some work before and after every method of the wrapped ValueExpression is called (e.g. LocationValueExpression for ValueExpressions containing #{cc}). Accessing the wrapper directly would change the expected behavior.

> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2621) BeanValidation does not work with Unified EL 2.2

Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12850899#action_12850899 ] 

Jakob Korherr commented on MYFACES-2621:
----------------------------------------

So this works now perfectly :)

The only thing which I really don't like now is that we are calling ExternalSpecifications.isUnifiedELAvailable() very often (for every ValueExpression in TagAttributeImpl at least once) and this method is synchronized. And frankly, I really don't understand why this one has to be synchronized. It is null at first and then it changes to Boolean.TRUE or Boolean.FALSE and is never changed again, as Leonardo wrote before. When we're removing synchronized the only thing which can happen is that more than one thread sets it from null to true or false, but this really is no problem, because every thread will come up with the same result here!

So removing synchronized should not cause any problems here or am I missing something? What were your concerns, Jan-Kees?



> BeanValidation does not work with Unified EL 2.2
> ------------------------------------------------
>
>                 Key: MYFACES-2621
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2621
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-3
>         Environment: bean validation 1.0.0.GA, unified el 2.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>
> When using the new Unified EL 2.2, BeanValidation does not work, because _BeanValidatorUELUtils.getUELValueReferenceWrapper() always returns null.
> See also the related thread on the mailing list: http://www.mail-archive.com/users@myfaces.apache.org/msg55250.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.