groovy
  1. groovy
  2. GROOVY-2503 MOP 2.0 design inflluencing issues
  3. GROOVY-3584

GroovyInterceptable should cause properties to fall back to getXXX/setXXX

    Details

    • Type: Sub-task Sub-task
    • Status: Open Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: groovy-runtime
    • Labels:
      None
    • Number of attachments :
      0

      Description

      If GroovyInterceptable is implemented, getProperty/setProperty should delegate to getXXX/setXXX method calls by default. If people have overridden getProperty/setProperty, they won't notice. In many cases, though, this will simplify meta-programming.

        Activity

        Hide
        blackdrag blackdrag added a comment -

        get/setProperty is used by default already. As an example:

        class Foo {
          def x = 1
          def getProperty(String name) {2}
        }
        def foo = new Foo()
        assert foo.x == 2
        

        so nothing to do?

        Show
        blackdrag blackdrag added a comment - get/setProperty is used by default already. As an example: class Foo { def x = 1 def getProperty( String name) {2} } def foo = new Foo() assert foo.x == 2 so nothing to do?
        Hide
        Robert Fischer added a comment -

        Ah, okay – I need to clarify.

        This code:

        class Recorder implements GroovyInterceptable {

        def toInvoke = []

        def invokeMethod(String name, args) {
        switch(name) {
        case "record":
        args[0].delegate = this
        args[0](this)
        break
        case "playback":
        toInvoke.each

        { methodName, methodArgs -> args[0].invokeMethod(methodName, methodArgs) }


        break
        default: toInvoke << [name, args]
        }
        }

        }

        def r = new Recorder()
        r.record

        { it.foo it.bar = 5 }

        Throws this exception:

        Caught: groovy.lang.MissingPropertyException: No such property: foo for class: Recorder
        at recorder_demo$_run_closure1.doCall(recorder_demo.groovy:25)
        at Recorder.invokeMethod(recorder_demo.groovy:10)
        at recorder_demo.run(recorder_demo.groovy:24)

        It can be fixed by adding this code to the Recorder:

        def getProperty(String propertyName)

        { toInvoke << ["getProperty", [propertyName]] }

        void setProperty(String propertyName, newValue)

        { toInvoke << ["setProperty", [propertyName, newValue]] }

        It'd be nice if that code didn't have to be added: if properties would eventually delegate back to invokeMethod, too.

        This request came out of code exploration here:
        http://enfranchisedmind.com/blog/posts/groovy-recorder/

        Show
        Robert Fischer added a comment - Ah, okay – I need to clarify. This code: class Recorder implements GroovyInterceptable { def toInvoke = [] def invokeMethod(String name, args) { switch(name) { case "record": args [0] .delegate = this args [0] (this) break case "playback": toInvoke.each { methodName, methodArgs -> args[0].invokeMethod(methodName, methodArgs) } break default: toInvoke << [name, args] } } } def r = new Recorder() r.record { it.foo it.bar = 5 } Throws this exception: Caught: groovy.lang.MissingPropertyException: No such property: foo for class: Recorder at recorder_demo$_run_closure1.doCall(recorder_demo.groovy:25) at Recorder.invokeMethod(recorder_demo.groovy:10) at recorder_demo.run(recorder_demo.groovy:24) It can be fixed by adding this code to the Recorder: def getProperty(String propertyName) { toInvoke << ["getProperty", [propertyName]] } void setProperty(String propertyName, newValue) { toInvoke << ["setProperty", [propertyName, newValue]] } It'd be nice if that code didn't have to be added: if properties would eventually delegate back to invokeMethod, too. This request came out of code exploration here: http://enfranchisedmind.com/blog/posts/groovy-recorder/
        Hide
        blackdrag blackdrag added a comment -

        so the problem is not that "println foo.bar" goes through getProperty(String) it is that the call to getProperty(String) does not go through invokeMethod, even though we have GroovyInterceptable. Well... currently you cannot even dynamically replace get/setProperty or invokeMethod combined with GroovyInterceptable. That is because a direct method call is done for them atm. Your proposed logic requires a dynamic method call to those methods. Which of course also means that invokeMethod will always have to handle get/setProperty too. So the maximum is to set this for 2.0

        Show
        blackdrag blackdrag added a comment - so the problem is not that "println foo.bar" goes through getProperty(String) it is that the call to getProperty(String) does not go through invokeMethod, even though we have GroovyInterceptable. Well... currently you cannot even dynamically replace get/setProperty or invokeMethod combined with GroovyInterceptable. That is because a direct method call is done for them atm. Your proposed logic requires a dynamic method call to those methods. Which of course also means that invokeMethod will always have to handle get/setProperty too. So the maximum is to set this for 2.0
        Hide
        Robert Fischer added a comment -

        My proposal would be for the default implementation of getProperty to end with something like:

        if(this instanceof GroovyInterceptable) {
          invokeMethod("getProperty", name)
        }
        

        And the default implementation of setProperty to end with something like:

        if(this instanceof GroovyInterceptable) {
          invokeMethod("setProperty", [name, value] as Object[])
        }
        

        (or whatever)

        So then, instead of throwing a MissingProperty exception, we're giving invokeMethod a chance to handle it.

        Show
        Robert Fischer added a comment - My proposal would be for the default implementation of getProperty to end with something like: if ( this instanceof GroovyInterceptable) { invokeMethod( "getProperty" , name) } And the default implementation of setProperty to end with something like: if ( this instanceof GroovyInterceptable) { invokeMethod( "setProperty" , [name, value] as Object []) } (or whatever) So then, instead of throwing a MissingProperty exception, we're giving invokeMethod a chance to handle it.
        Hide
        Robert Fischer added a comment -

        I assumed that this wouldn't be targeted for anything less than 2.0 (hence the "Improvement", not "Bug" categorization).

        Show
        Robert Fischer added a comment - I assumed that this wouldn't be targeted for anything less than 2.0 (hence the "Improvement", not "Bug" categorization).
        Hide
        blackdrag blackdrag added a comment -

        wait... you want the method to be called to call itself? I mean interception means to get to a point where the method call has been initiated, but the actual method is not yet called. For getProperty(String) that for example means to react to that method call before the method is actually called. If the method just calls itself, you end up with a endless loop or stack overflow. You try to limit this by checking for GroovyInterceptable, but what should invokeMethod then call? If it tries to actually call getProperty you end up in invokeMethod again! I think this won't do.

        btw, using a custom meta class would be much better for this kind of stuff.

        Show
        blackdrag blackdrag added a comment - wait... you want the method to be called to call itself? I mean interception means to get to a point where the method call has been initiated, but the actual method is not yet called. For getProperty(String) that for example means to react to that method call before the method is actually called. If the method just calls itself, you end up with a endless loop or stack overflow. You try to limit this by checking for GroovyInterceptable, but what should invokeMethod then call? If it tries to actually call getProperty you end up in invokeMethod again! I think this won't do. btw, using a custom meta class would be much better for this kind of stuff.
        Hide
        Robert Fischer added a comment -

        Right now, properties aren't handled at all by invokeMethod: calls to properties aren't intercepted by GroovyInterceptable at all.

        I do see the circularity issue, though: if neither invokeMethod nor getProperty/setProperty oare overridden to handle it, you'd end up with an explosion.

        How is a custom MetaClass any better? Seems a lot more complicated, and I'm not sure what's gained.

        Show
        Robert Fischer added a comment - Right now, properties aren't handled at all by invokeMethod: calls to properties aren't intercepted by GroovyInterceptable at all. I do see the circularity issue, though: if neither invokeMethod nor getProperty/setProperty oare overridden to handle it, you'd end up with an explosion. How is a custom MetaClass any better? Seems a lot more complicated, and I'm not sure what's gained.
        Hide
        blackdrag blackdrag added a comment -

        get/setProperty are handled by invokeMethod is they are called as methods. So "not at all" seems to exaggerate a bit. I only wanted to hint, that this way to implement the methods doesn't help very much.

        How a MetaClass is better? Well, not so much for Interceptable, but normally you don't want to change the static class delcaration to record. So IMHO this goes more into Mocking and Stubbing. And here Groovy has MockFor and StubFor and those use ProxyMetaClass to accomplish their task. Using ProxyMetaClass is much less complicated than a full blown meta class of course.

        Show
        blackdrag blackdrag added a comment - get/setProperty are handled by invokeMethod is they are called as methods. So "not at all" seems to exaggerate a bit. I only wanted to hint, that this way to implement the methods doesn't help very much. How a MetaClass is better? Well, not so much for Interceptable, but normally you don't want to change the static class delcaration to record. So IMHO this goes more into Mocking and Stubbing. And here Groovy has MockFor and StubFor and those use ProxyMetaClass to accomplish their task. Using ProxyMetaClass is much less complicated than a full blown meta class of course.

          People

          • Assignee:
            Unassigned
            Reporter:
            Robert Fischer
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated: