Commons Digester
  1. Commons Digester
  2. DIGESTER-71

CallMethodRule rule can't be used to call no arg methods

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

      Description

      The digester's CallMethodRule rule can't be used to call methods taking no
      argument. The main issue is the semantic of the constructor, a paramCount of 0
      doesn't stand for "no argument" but for "pass the tag body as argument", thus
      preventing the call of no arg methods on the parent objet.

      One could expect CallMethodRule to behave like Class.getMethod(), that is, a
      null or empty Class[] array means we are looking for a no argument method.
      Currently the decision is not made on the value of paramTypes but on the value
      of paramCount. If paramType is empty or null it's forced to an array with a
      single String.class element :

      in the constructor :

      if (paramTypes == null) {
      this.paramTypes = new Class[paramCount];

      and later in end() :

      if (paramTypes.length == 0)

      { paramTypes = new Class[1]; paramTypes[0] = "abc".getClass(); }

      Fixing this issue is just a matter of letting paramTypes untouched till the
      method.invoke() call.

      1. ASF.LICENSE.NOT.GRANTED--CallMethodRule-noargfix.txt
        7 kB
        Emmanuel Bourg
      2. ASF.LICENSE.NOT.GRANTED--Test5.xml
        0.4 kB
        Emmanuel Bourg
      3. ASF.LICENSE.NOT.GRANTED--tmp.txt
        2 kB
        Michael Marrotte

        Activity

        Hide
        robertdonkin@mac.com added a comment -

        i've added support for calls to empty methods to the xmlrules.

        thanks to Michael Marrotte for providing a patch but i prefered to commit an alternative
        solution (for stylist reasons). the dtd has been loosened slightly to allow just a method
        name attribute to be specified. when only a method name is specified, then this should call
        an empty method.

        see test cases for an example.

        please give this fix a whirl if you get the time.

        Show
        robertdonkin@mac.com added a comment - i've added support for calls to empty methods to the xmlrules. thanks to Michael Marrotte for providing a patch but i prefered to commit an alternative solution (for stylist reasons). the dtd has been loosened slightly to allow just a method name attribute to be specified. when only a method name is specified, then this should call an empty method. see test cases for an example. please give this fix a whirl if you get the time.
        Hide
        Michael Marrotte added a comment -

        Great... But, what about xmlrules? For the "*/call-method-rule" pattern,
        DigesterRuleParser adds a CallMethodRuleFactory who's createObject returns a
        CallMethodRule that does not handle the paramCount=0 consistent to this fix.

        Therefore xmlrules users run into the same old problem.

        I attached another fix.

        Show
        Michael Marrotte added a comment - Great... But, what about xmlrules? For the "*/call-method-rule" pattern, DigesterRuleParser adds a CallMethodRuleFactory who's createObject returns a CallMethodRule that does not handle the paramCount=0 consistent to this fix. Therefore xmlrules users run into the same old problem. I attached another fix.
        Hide
        Michael Marrotte added a comment -

        Created an attachment (id=2228)
        DigesterRuleParser patch

        Show
        Michael Marrotte added a comment - Created an attachment (id=2228) DigesterRuleParser patch
        Hide
        robertdonkin@mac.com added a comment -

        i've committed this patch. (with the addition of an easy constructor.)

        a very well thought through solution to a potentially tricky situation. i've convinced
        myself that this should be backwards compatible for all realistic applications.

        many thanks for a well prepared and very document patch.

        • robert
        Show
        robertdonkin@mac.com added a comment - i've committed this patch. (with the addition of an easy constructor.) a very well thought through solution to a potentially tricky situation. i've convinced myself that this should be backwards compatible for all realistic applications. many thanks for a well prepared and very document patch. robert
        Hide
        Emmanuel Bourg added a comment -

        Here is the patch i suggest. There was no test case for the CallMethodRule rule
        with paramCount=0 so i added one in RuleTestCase.java. I also added a test for
        CallMethodRule when calling a method taking no arguments, I tested with
        toString() on the class Employee. Both tests use a new XML test file, Test5.xml.

        CallMethodRule.java has been changed so that a no argument method is called when
        paramCount is set to zero and paramTypes is null or empty.

        Javadocs in Digester.java and CallMethodRule.java have been updated accordingly.

        This patch will break uses of CallMethodRule with paramCount=0 AND
        paramTypes=null (or empty). However nobody should have used the rule this way,
        it's a nonsense. In this case paramTypes provides no useful information and
        could be omitted.

        Show
        Emmanuel Bourg added a comment - Here is the patch i suggest. There was no test case for the CallMethodRule rule with paramCount=0 so i added one in RuleTestCase.java. I also added a test for CallMethodRule when calling a method taking no arguments, I tested with toString() on the class Employee. Both tests use a new XML test file, Test5.xml. CallMethodRule.java has been changed so that a no argument method is called when paramCount is set to zero and paramTypes is null or empty. Javadocs in Digester.java and CallMethodRule.java have been updated accordingly. This patch will break uses of CallMethodRule with paramCount=0 AND paramTypes=null (or empty). However nobody should have used the rule this way, it's a nonsense. In this case paramTypes provides no useful information and could be omitted.
        Hide
        Emmanuel Bourg added a comment -

        Created an attachment (id=1615)
        Test5.xml test file used by testObjectCreate5 and testCallMethod

        Show
        Emmanuel Bourg added a comment - Created an attachment (id=1615) Test5.xml test file used by testObjectCreate5 and testCallMethod
        Hide
        Emmanuel Bourg added a comment -

        Created an attachment (id=1614)
        Fix + testcases & doc

        Show
        Emmanuel Bourg added a comment - Created an attachment (id=1614) Fix + testcases & doc

          People

          • Assignee:
            Unassigned
            Reporter:
            Emmanuel Bourg
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development