MyFaces Core
  1. MyFaces Core
  2. MYFACES-2565

BeanValidator throws Exception if external ExpressionLanguageFactory is being used

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-beta
    • Fix Version/s: 2.0.0-beta-3
    • Component/s: None
    • Labels:
      None

      Description

      Currently the BeanValidator has the following code:

      if (_ExternalSpecifications.isUnifiedELAvailable())

      { //TODO: Implement when Unified EL for Java EE6 is available. throw new FacesException("Unified EL for Java EE6 support is not yet implemented"); }

      I'm using EL-2.2 in MyFaces-2.0.0-SNAPSHOT and now switched from using facelets-1.1.15.B1 to the built-in facelets-2.

      After that I get this rather unfunny Exception in the code above.

      I tried to remove this condition in BeanValidator and so far all works well. Afaik the new EL-2.1 interface should be backward compatible to the EL of JSF-2.1, isn't?

        Activity

        Hide
        Jan-Kees van Andel added a comment -

        I put it there when implementing the first version of BeanValidator. The spec says the following:
        http://java.sun.com/javaee/javaserverfaces/2.0/docs/api/javax/faces/validator/BeanValidator.html#validate%28javax.faces.context.FacesContext,%20javax.faces.component.UIComponent,%20java.lang.Object%29

        Let valueExpression be the return from calling UIComponent.getValueExpression(java.lang.String) on the argument component, passing the literal string "value" (without the quotes) as an argument. If this application is running in an environment with a Unified EL Implementation for Java EE6 or later, obtain the ValueReference from valueExpression and let valueBaseClase be the return from calling ValueReference.getBase() and valueProperty be the return from calling ValueReference.getProperty(). If an earlier version of the Unified EL is present, use the appropriate methods to inspect valueExpression and derive values for valueBaseClass and valueProperty.

        Back then, it was not possible to completely implement this method, because Unified EL was not available, so I put the TODO and Exception in. I assume we can now safely implement the remaining part.

        Show
        Jan-Kees van Andel added a comment - I put it there when implementing the first version of BeanValidator. The spec says the following: http://java.sun.com/javaee/javaserverfaces/2.0/docs/api/javax/faces/validator/BeanValidator.html#validate%28javax.faces.context.FacesContext,%20javax.faces.component.UIComponent,%20java.lang.Object%29 Let valueExpression be the return from calling UIComponent.getValueExpression(java.lang.String) on the argument component, passing the literal string "value" (without the quotes) as an argument. If this application is running in an environment with a Unified EL Implementation for Java EE6 or later, obtain the ValueReference from valueExpression and let valueBaseClase be the return from calling ValueReference.getBase() and valueProperty be the return from calling ValueReference.getProperty(). If an earlier version of the Unified EL is present, use the appropriate methods to inspect valueExpression and derive values for valueBaseClass and valueProperty. Back then, it was not possible to completely implement this method, because Unified EL was not available, so I put the TODO and Exception in. I assume we can now safely implement the remaining part.
        Hide
        Mark Struberg added a comment -

        found an issue which may be related with commenting out the section above:

        In some very rare cases I crash in

        Class<?> valueBaseClass = valueReferenceWrapper.getBase().getClass();

        because getBase() returns null.

        I have no clue why this can happen, but may have something to do with my changes above.

        So maybe the "TODO" is likely a bit more work

        Show
        Mark Struberg added a comment - found an issue which may be related with commenting out the section above: In some very rare cases I crash in Class<?> valueBaseClass = valueReferenceWrapper.getBase().getClass(); because getBase() returns null. I have no clue why this can happen, but may have something to do with my changes above. So maybe the "TODO" is likely a bit more work
        Hide
        Jan-Kees van Andel added a comment -

        Just looking at the JavaDocs of BeanValidator#validate(), it looks like a null-check is missing.

        "If ValueReference.getBase() return null, take no action and return."

        I'll try to fix this issue (as well as the other BeanValidator related issues) this weekend, but it depends, since I'm a bit behind schedule after JSFDays...

        Show
        Jan-Kees van Andel added a comment - Just looking at the JavaDocs of BeanValidator#validate(), it looks like a null-check is missing. "If ValueReference.getBase() return null, take no action and return." I'll try to fix this issue (as well as the other BeanValidator related issues) this weekend, but it depends, since I'm a bit behind schedule after JSFDays...
        Hide
        Mark Struberg added a comment -

        oki, then it seems I at least locally patched it the right way already -txs.
        And don't worry, I think JSFdays was a big boost again in terms of ideas, knowledge and of course motivation which will overcompensate loosing those 3 days easily

        Show
        Mark Struberg added a comment - oki, then it seems I at least locally patched it the right way already -txs. And don't worry, I think JSFdays was a big boost again in terms of ideas, knowledge and of course motivation which will overcompensate loosing those 3 days easily
        Hide
        Jan-Kees van Andel added a comment -

        It should work now, however I don't like the implementation. I need to resort to reflection if I want to use the ValueExpression.getValueReference() method, because the compile classpath contains two entries for ValueExpression: One in jsp-api-2.1 and one in el-api-2.2.

        Someone please review the implementation...

        Show
        Jan-Kees van Andel added a comment - It should work now, however I don't like the implementation. I need to resort to reflection if I want to use the ValueExpression.getValueReference() method, because the compile classpath contains two entries for ValueExpression: One in jsp-api-2.1 and one in el-api-2.2. Someone please review the implementation...
        Hide
        Jakob Korherr added a comment -

        ExternalSpecifications is also present in myfaces-impl (/myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/util/ExternalSpecifications.java). Please also apply your changes there, Jan-Kees

        Show
        Jakob Korherr added a comment - ExternalSpecifications is also present in myfaces-impl (/myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/util/ExternalSpecifications.java). Please also apply your changes there, Jan-Kees
        Hide
        Jan-Kees van Andel added a comment -

        Okay, but it's getting a bit nasty this way (copy-pasting this class 3 times, especially since it is getting more complex).

        I assume we also need to work with Java access permissions turned on? Otherwise, we can do some reflection stuff, like:

        • Putting all logic inside one class, let's say: javax.faces.validate._ExternalSpecifications
        • Putting some kind of bridge classes in other packages, like:

        package javax.faces.component;
        class _ExternalSpecificationsBridge {

        public boolean isUnifiedELAvailable() {
        try

        { Class c = Class.forName("javax.faces.validate._ExternalSpecifications"); Method m = c.getMethod("isUnifiedELAvailable"); m.setAccessible(true); return Boolean.TRUE.equals(m.invoke()); }

        catch ()

        { return false; }

        }

        }

        Show
        Jan-Kees van Andel added a comment - Okay, but it's getting a bit nasty this way (copy-pasting this class 3 times, especially since it is getting more complex). I assume we also need to work with Java access permissions turned on? Otherwise, we can do some reflection stuff, like: Putting all logic inside one class, let's say: javax.faces.validate._ExternalSpecifications Putting some kind of bridge classes in other packages, like: package javax.faces.component; class _ExternalSpecificationsBridge { public boolean isUnifiedELAvailable() { try { Class c = Class.forName("javax.faces.validate._ExternalSpecifications"); Method m = c.getMethod("isUnifiedELAvailable"); m.setAccessible(true); return Boolean.TRUE.equals(m.invoke()); } catch () { return false; } } }
        Hide
        Jakob Korherr added a comment -

        I know it's not nice to have this replication (I hate it too), but it's the fasted solution under our condition.

        Another possible solution would be to evaluate the fields from ExternalSpecifications in the ServletContextListener and store them under a specific key in the application map. This way we would get rid of our 3 implementations...

        Show
        Jakob Korherr added a comment - I know it's not nice to have this replication (I hate it too), but it's the fasted solution under our condition. Another possible solution would be to evaluate the fields from ExternalSpecifications in the ServletContextListener and store them under a specific key in the application map. This way we would get rid of our 3 implementations...
        Hide
        Jan-Kees van Andel added a comment -

        Separate issue created, see: MYFACES-2582

        Show
        Jan-Kees van Andel added a comment - Separate issue created, see: MYFACES-2582
        Hide
        Jakob Korherr added a comment -

        ExternalSpecifications on myfaces-impl must not be package private, because it is used in many packages of myfaces-impl! I'll change this.

        Show
        Jakob Korherr added a comment - ExternalSpecifications on myfaces-impl must not be package private, because it is used in many packages of myfaces-impl! I'll change this.
        Hide
        Leonardo Uribe added a comment -

        The patch committed here causes MYFACES-2605. It was added these dependencies:

        <dependency>
        <groupId>javax.validation</groupId>
        <artifactId>validation-api</artifactId>
        <version>1.0.CR3</version>
        <optional>true</optional>
        </dependency>

        <dependency>
        <groupId>javax.el</groupId>
        <artifactId>el-api</artifactId>
        <version>2.1.2-b05</version>
        <optional>true</optional>
        </dependency>

        That's not good. The el-api dependency version should be 1.0, so we can't use code in a upper version.

        Show
        Leonardo Uribe added a comment - The patch committed here causes MYFACES-2605 . It was added these dependencies: <dependency> <groupId>javax.validation</groupId> <artifactId>validation-api</artifactId> <version>1.0.CR3</version> <optional>true</optional> </dependency> <dependency> <groupId>javax.el</groupId> <artifactId>el-api</artifactId> <version>2.1.2-b05</version> <optional>true</optional> </dependency> That's not good. The el-api dependency version should be 1.0, so we can't use code in a upper version.

          People

          • Assignee:
            Leonardo Uribe
            Reporter:
            Mark Struberg
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development