Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.3.0-M1
    • Component/s: None
    • Labels:
      None

      Description

      Remove OgnlOps dependency from core. The reason for this class inclusion is because of a bug in BigDecimal parsing. This code snippet could be included in Click's RequestTypeConverter instead.

        Activity

        Hide
        medgar Malcolm Edgar added a comment -

        This was introduced to fix a bug performing a form copy operation, not a request binding operation from what I recall.

        I think we should revisit using OGNL and see if MVEL or something else is more appropriate. However at this point I would not recommend removing this code, as it would probably reintroduce this bug.

        regards Malcolm Edgar

        Show
        medgar Malcolm Edgar added a comment - This was introduced to fix a bug performing a form copy operation, not a request binding operation from what I recall. I think we should revisit using OGNL and see if MVEL or something else is more appropriate. However at this point I would not recommend removing this code, as it would probably reintroduce this bug. regards Malcolm Edgar
        Hide
        a_adrian Adrian A. added a comment -

        > I think we should revisit using OGNL and see if MVEL or something else is more appropriate.
        MVEL is much bigger than OGNL, and the the javassist dependency scope of the newer OGNL versions is "provided", so it shouldn't be a problem.
        Also since OGNL is in Click since ages, many Click webapplications made use of it and now depend on it greatly

        IMHO we could also safely upgrade to OGNL 3.0 https://issues.apache.org/jira/browse/CLK-672

        Show
        a_adrian Adrian A. added a comment - > I think we should revisit using OGNL and see if MVEL or something else is more appropriate. MVEL is much bigger than OGNL, and the the javassist dependency scope of the newer OGNL versions is "provided", so it shouldn't be a problem. Also since OGNL is in Click since ages, many Click webapplications made use of it and now depend on it greatly IMHO we could also safely upgrade to OGNL 3.0 https://issues.apache.org/jira/browse/CLK-672
        Hide
        sabob Bob Schellink added a comment -

        As explained above we cannot remove OgnlOps dependency. However we should still look at an alternative for OGNL. MVEL[1] is one candidate and JUEL[2] looks interesting too. Not sure what the performance of JUEL is, however it was defined as a standard jsr245 [3].

        [1]: http://mvel.codehaus.org/
        [2]: http://juel.sourceforge.net/
        [3]: http://jcp.org/aboutJava/communityprocess/final/jsr245/

        Show
        sabob Bob Schellink added a comment - As explained above we cannot remove OgnlOps dependency. However we should still look at an alternative for OGNL. MVEL [1] is one candidate and JUEL [2] looks interesting too. Not sure what the performance of JUEL is, however it was defined as a standard jsr245 [3] . [1] : http://mvel.codehaus.org/ [2] : http://juel.sourceforge.net/ [3] : http://jcp.org/aboutJava/communityprocess/final/jsr245/
        Hide
        sabob Bob Schellink added a comment -

        Finn did in fact implement this by injecting a custom converter into the OGNL context.

        Show
        sabob Bob Schellink added a comment - Finn did in fact implement this by injecting a custom converter into the OGNL context.
        Hide
        sabob Bob Schellink added a comment -

        fixed in trunk

        Show
        sabob Bob Schellink added a comment - fixed in trunk

          People

          • Assignee:
            sabob Bob Schellink
            Reporter:
            sabob Bob Schellink
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development