Commons Digester
  1. Commons Digester
  2. DIGESTER-31

[digester] XML Rule for stack param takes useless boolean instead of int arg

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.6
    • Fix Version/s: 1.7
    • Labels:
      None
    • Environment:

      Operating System: Linux
      Platform: PC

      Description

      The XML version of a method parameter rule, to take an argument from the stack,
      takes a boolean "true" or "false" as argument. This means that it is possible,
      only, to specify the top of the stack as a parameter. Since the eventual method
      call is always made to the object on top of the stack, this is particularly
      useless. If the argument were, instead, an integer specifying the stack offset,
      the XML rule creation would have the same functionality as the programatic analogue.

        Activity

        Hide
        Simon Kitching added a comment -

        Hi Blake,

        After some thought, I did feel it better to have a separate xml attribute rather
        than overload the "from-stack" one. Kris Nuttycombe had already provided a patch
        that did exactly this (see bugzilla#32891) after seeing your original request
        for this feature, so I have committed Kris' patch.

        Thanks very much for your contribution, and I hope we see more of you on
        commons-dev.

        Regards,

        Simon

        Show
        Simon Kitching added a comment - Hi Blake, After some thought, I did feel it better to have a separate xml attribute rather than overload the "from-stack" one. Kris Nuttycombe had already provided a patch that did exactly this (see bugzilla#32891) after seeing your original request for this feature, so I have committed Kris' patch. Thanks very much for your contribution, and I hope we see more of you on commons-dev. Regards, Simon
        Hide
        Simon Kitching added a comment -

        Hi Blake/Kris,

        I haven't forgotten this and #32891 (both related). I will try to have a look
        very soon now...

        Show
        Simon Kitching added a comment - Hi Blake/Kris, I haven't forgotten this and #32891 (both related). I will try to have a look very soon now...
        Hide
        Blake Meike added a comment -

        Created an attachment (id=13941)
        Patch adds integer stack offset param rule

        Points taken.

        I've enclosed a patch that adds the new functionality, by overloading the
        existing attribute. I can imagine prefering an implementation that uses a
        distinct attribute for the new functionality. Let me know if that would be
        better.

        Show
        Blake Meike added a comment - Created an attachment (id=13941) Patch adds integer stack offset param rule Points taken. I've enclosed a patch that adds the new functionality, by overloading the existing attribute. I can imagine prefering an implementation that uses a distinct attribute for the new functionality. Let me know if that would be better.
        Hide
        Simon Kitching added a comment -

        The version of CallParamRule that takes a boolean is not useless at all.

        It stores the object which is on top of the stack *at the time the CallParamRule
        fires* for passing to a target method. The target method will be on the object
        which is on top of the stack at the time the CallMethodRule fires.

        Below is some code from the digester unit tests which uses CallParamRule(int,
        boolean) in the usual manner.

        The code which allows CallParamRule to take an arbitrary stack offset was added
        later, and is really only needed for unusual cases. However you are right that
        it should be accessable via the xmlrules module. Would you be interested in
        providing a patch for xmlrules that adds this extra functionality? (rather
        than changing the existing behaviour, which is not a good idea as it will break
        existing programs that use CallParamRule(int, bool) or its xmlrules mapping).

        Code from digester unit tests:

        /**

        • This tests the call methods params enhancement that provides
        • for more complex stack-based calls.
          */
          public void testParamsFromStack() throws SAXException, IOException { StringBuffer xml = new StringBuffer(). append("<?xml version='1.0'?>"). append("<map>"). append(" <key name='The key'/>"). append(" <value name='The value'/>"). append("</map>"); digester.addObjectCreate("map", HashMap.class); digester.addCallMethod("map", "put", 2); digester.addObjectCreate("map/key", AlphaBean.class); digester.addSetProperties("map/key"); digester.addCallParam("map/key", 0, true); digester.addObjectCreate("map/value", BetaBean.class); digester.addSetProperties("map/value"); digester.addCallParam("map/value", 1, true); Map map = (Map) digester.parse(new StringReader(xml.toString())); assertNotNull(map); assertEquals(1, map.size()); assertEquals("The key", ((AlphaBean)map.keySet().iterator().next()).getName()); assertEquals("The value", ((BetaBean)map.values().iterator().next()).getName()); }
        Show
        Simon Kitching added a comment - The version of CallParamRule that takes a boolean is not useless at all. It stores the object which is on top of the stack *at the time the CallParamRule fires* for passing to a target method. The target method will be on the object which is on top of the stack at the time the CallMethodRule fires . Below is some code from the digester unit tests which uses CallParamRule(int, boolean) in the usual manner. The code which allows CallParamRule to take an arbitrary stack offset was added later, and is really only needed for unusual cases. However you are right that it should be accessable via the xmlrules module. Would you be interested in providing a patch for xmlrules that adds this extra functionality? (rather than changing the existing behaviour, which is not a good idea as it will break existing programs that use CallParamRule(int, bool) or its xmlrules mapping). Code from digester unit tests: /** This tests the call methods params enhancement that provides for more complex stack-based calls. */ public void testParamsFromStack() throws SAXException, IOException { StringBuffer xml = new StringBuffer(). append("<?xml version='1.0'?>"). append("<map>"). append(" <key name='The key'/>"). append(" <value name='The value'/>"). append("</map>"); digester.addObjectCreate("map", HashMap.class); digester.addCallMethod("map", "put", 2); digester.addObjectCreate("map/key", AlphaBean.class); digester.addSetProperties("map/key"); digester.addCallParam("map/key", 0, true); digester.addObjectCreate("map/value", BetaBean.class); digester.addSetProperties("map/value"); digester.addCallParam("map/value", 1, true); Map map = (Map) digester.parse(new StringReader(xml.toString())); assertNotNull(map); assertEquals(1, map.size()); assertEquals("The key", ((AlphaBean)map.keySet().iterator().next()).getName()); assertEquals("The value", ((BetaBean)map.values().iterator().next()).getName()); }
        Hide
        Blake Meike added a comment -

        Created an attachment (id=13880)
        Trivial fix

        Trivial fix.

        Show
        Blake Meike added a comment - Created an attachment (id=13880) Trivial fix Trivial fix.

          People

          • Assignee:
            Unassigned
            Reporter:
            Blake Meike
          • Votes:
            1 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development