groovy
  1. groovy
  2. GROOVY-3942

Using metaClass to override methods in class hierarchy does not work as expected

    Details

    • Type: Bug Bug
    • Status: Open Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.8-beta-1
    • Fix Version/s: None
    • Component/s: groovy-runtime
    • Labels:
      None
    • Environment:
      Windows XP
    • Number of attachments :
      1

      Description

      I've found some strange behaviour when dynamically overriding methods that are called in a class hierarchy, which I think might be a bug. The problem is that when I override a method on a subclass, code in the superclass still calls the original method (note all classes are Groovy ones, I know this wouldn't work with a Java class hierarchy).

      I've isolated it to this sample code:

          class BaseClass {
              def baseClassMethod() {
                  println "BaseClass.baseClassMethod()"
                  internalMethod()
              }
              def internalMethod() {
                 println "    BaseClass.internalMethod()"
              }
          }
      
          class SubClass extends BaseClass {
              def subClassMethod() {
                  println "SubClass.subClassMethod()"
                  internalMethod()
              }
          }
      
          def subClass = new SubClass()
          subClass.metaClass.internalMethod = { -> println ("    (dynamic).internalMethod()")}
          subClass.baseClassMethod()
          subClass.subClassMethod()
      

      ...which gives me the following output...

      BaseClass.baseClassMethod()
      BaseClass.internalMethod()
      SubClass.subClassMethod()
      (dynamic).internalMethod()

      I would have expected that the dynamic version of internalMethod() would be called both times, given that Groovy has dynamic method dispatching.

      Some discussion of this issue took place on the mailing list: http://old.nabble.com/Problems-using-metaClass-to-override-methods-in-class-hierarchy-td26743895.html#a26743895

      The suggestion is that the CallSiteArray.createCallCurrentSite(...) method could check whether the receiver is a sub class of the sender class.

        Activity

        Hide
        blackdrag blackdrag added a comment -

        added code tags

        Show
        blackdrag blackdrag added a comment - added code tags
        blackdrag blackdrag made changes -
        Field Original Value New Value
        Description I've found some strange behaviour when dynamically overriding methods that are called in a class hierarchy, which I think might be a bug. The problem is that when I override a method on a subclass, code in the superclass still calls the original method (note all classes are Groovy ones, I know this wouldn't work with a Java class hierarchy).

        I've isolated it to this sample code:

            class BaseClass {
                def baseClassMethod() {
                    println "BaseClass.baseClassMethod()"
                    internalMethod()
                }
                def internalMethod() {
                   println " BaseClass.internalMethod()"
                }
            }

            class SubClass extends BaseClass {
                def subClassMethod() {
                    println "SubClass.subClassMethod()"
                    internalMethod()
                }
            }

            def subClass = new SubClass()
            subClass.metaClass.internalMethod = { -> println (" (dynamic).internalMethod()")}
            subClass.baseClassMethod()
            subClass.subClassMethod()

        ...which gives me the following output...

            BaseClass.baseClassMethod()
                BaseClass.internalMethod()
            SubClass.subClassMethod()
                (dynamic).internalMethod()

        I would have expected that the dynamic version of internalMethod() would be called both times, given that Groovy has dynamic method dispatching.

        Some discussion of this issue took place on the mailing list: http://old.nabble.com/Problems-using-metaClass-to-override-methods-in-class-hierarchy-td26743895.html#a26743895

        The suggestion is that the CallSiteArray.createCallCurrentSite(...) method could check whether the receiver is a sub class of the sender class.
        I've found some strange behaviour when dynamically overriding methods that are called in a class hierarchy, which I think might be a bug. The problem is that when I override a method on a subclass, code in the superclass still calls the original method (note all classes are Groovy ones, I know this wouldn't work with a Java class hierarchy).

        I've isolated it to this sample code:
        {code:Java}
            class BaseClass {
                def baseClassMethod() {
                    println "BaseClass.baseClassMethod()"
                    internalMethod()
                }
                def internalMethod() {
                   println " BaseClass.internalMethod()"
                }
            }

            class SubClass extends BaseClass {
                def subClassMethod() {
                    println "SubClass.subClassMethod()"
                    internalMethod()
                }
            }

            def subClass = new SubClass()
            subClass.metaClass.internalMethod = { -> println (" (dynamic).internalMethod()")}
            subClass.baseClassMethod()
            subClass.subClassMethod()
        {ocde}
        ...which gives me the following output...

            BaseClass.baseClassMethod()
                BaseClass.internalMethod()
            SubClass.subClassMethod()
                (dynamic).internalMethod()

        I would have expected that the dynamic version of internalMethod() would be called both times, given that Groovy has dynamic method dispatching.

        Some discussion of this issue took place on the mailing list: http://old.nabble.com/Problems-using-metaClass-to-override-methods-in-class-hierarchy-td26743895.html#a26743895

        The suggestion is that the CallSiteArray.createCallCurrentSite(...) method could check whether the receiver is a sub class of the sender class.
        blackdrag blackdrag made changes -
        Description I've found some strange behaviour when dynamically overriding methods that are called in a class hierarchy, which I think might be a bug. The problem is that when I override a method on a subclass, code in the superclass still calls the original method (note all classes are Groovy ones, I know this wouldn't work with a Java class hierarchy).

        I've isolated it to this sample code:
        {code:Java}
            class BaseClass {
                def baseClassMethod() {
                    println "BaseClass.baseClassMethod()"
                    internalMethod()
                }
                def internalMethod() {
                   println " BaseClass.internalMethod()"
                }
            }

            class SubClass extends BaseClass {
                def subClassMethod() {
                    println "SubClass.subClassMethod()"
                    internalMethod()
                }
            }

            def subClass = new SubClass()
            subClass.metaClass.internalMethod = { -> println (" (dynamic).internalMethod()")}
            subClass.baseClassMethod()
            subClass.subClassMethod()
        {ocde}
        ...which gives me the following output...

            BaseClass.baseClassMethod()
                BaseClass.internalMethod()
            SubClass.subClassMethod()
                (dynamic).internalMethod()

        I would have expected that the dynamic version of internalMethod() would be called both times, given that Groovy has dynamic method dispatching.

        Some discussion of this issue took place on the mailing list: http://old.nabble.com/Problems-using-metaClass-to-override-methods-in-class-hierarchy-td26743895.html#a26743895

        The suggestion is that the CallSiteArray.createCallCurrentSite(...) method could check whether the receiver is a sub class of the sender class.
        I've found some strange behaviour when dynamically overriding methods that are called in a class hierarchy, which I think might be a bug. The problem is that when I override a method on a subclass, code in the superclass still calls the original method (note all classes are Groovy ones, I know this wouldn't work with a Java class hierarchy).

        I've isolated it to this sample code:
        {code:Java}
            class BaseClass {
                def baseClassMethod() {
                    println "BaseClass.baseClassMethod()"
                    internalMethod()
                }
                def internalMethod() {
                   println " BaseClass.internalMethod()"
                }
            }

            class SubClass extends BaseClass {
                def subClassMethod() {
                    println "SubClass.subClassMethod()"
                    internalMethod()
                }
            }

            def subClass = new SubClass()
            subClass.metaClass.internalMethod = { -> println (" (dynamic).internalMethod()")}
            subClass.baseClassMethod()
            subClass.subClassMethod()
        {code}
        ...which gives me the following output...

            BaseClass.baseClassMethod()
                BaseClass.internalMethod()
            SubClass.subClassMethod()
                (dynamic).internalMethod()

        I would have expected that the dynamic version of internalMethod() would be called both times, given that Groovy has dynamic method dispatching.

        Some discussion of this issue took place on the mailing list: http://old.nabble.com/Problems-using-metaClass-to-override-methods-in-class-hierarchy-td26743895.html#a26743895

        The suggestion is that the CallSiteArray.createCallCurrentSite(...) method could check whether the receiver is a sub class of the sender class.
        Hide
        blackdrag blackdrag added a comment -

        there is a point the meta class is asked for the method, but in this case the wrong meta class is used. Instead of using the meta class of BaseClass it must use the one of SubClass. This meta class is used initially for searching for the method the first time the method is invoked. There is a check on the call site, that invalidates it, if something is wrong. And that SubClass is not BaseClass should be such a condition.

        Show
        blackdrag blackdrag added a comment - there is a point the meta class is asked for the method, but in this case the wrong meta class is used. Instead of using the meta class of BaseClass it must use the one of SubClass. This meta class is used initially for searching for the method the first time the method is invoked. There is a check on the call site, that invalidates it, if something is wrong. And that SubClass is not BaseClass should be such a condition.
        Hide
        blackdrag blackdrag added a comment -

        I schedule it for 1.8 for now

        Show
        blackdrag blackdrag added a comment - I schedule it for 1.8 for now
        blackdrag blackdrag made changes -
        Affects Version/s 1.6.2 [ 15151 ]
        Affects Version/s 1.8-beta-1 [ 16013 ]
        Hide
        Jason Griffith added a comment -

        Hi everyone.

        We're having some problems using metaClass to override methods in a subclass for unit testing, and I think it's related to this problem.

        Attached are some unit tests that illustrate the issue. We have an abstract class that we want to test, and we use a test wrapper class to help out. We can't override the abstract class' methods effectively because any calls within the base class ignore metaClass, unless the code is executed within a Closure.

        So there's an ugly, ugly workaround at least...

        Show
        Jason Griffith added a comment - Hi everyone. We're having some problems using metaClass to override methods in a subclass for unit testing, and I think it's related to this problem. Attached are some unit tests that illustrate the issue. We have an abstract class that we want to test, and we use a test wrapper class to help out. We can't override the abstract class' methods effectively because any calls within the base class ignore metaClass, unless the code is executed within a Closure. So there's an ugly, ugly workaround at least...
        Hide
        Jason Griffith added a comment -

        Unit Tests to illustrate issue

        Show
        Jason Griffith added a comment - Unit Tests to illustrate issue
        Jason Griffith made changes -
        Attachment MetaclassOverrideTest.groovy [ 58074 ]
        Hide
        John Troxel added a comment -

        I like to use metaClass to stub the class under test, especially if I didn't write it--methods that the method I am testing call in the same object. However I have bumped my head on this exact issue a few times, in which case I end up using an anonymous subclass.

        Show
        John Troxel added a comment - I like to use metaClass to stub the class under test, especially if I didn't write it--methods that the method I am testing call in the same object. However I have bumped my head on this exact issue a few times, in which case I end up using an anonymous subclass.
        Hide
        marko.m added a comment -

        I think this is same issue that is responsible for the strange behavior i posted to the groovy user list?
        http://groovy.329449.n5.nabble.com/should-this-method-override-work-td5566431.html

        This feel like serious issue, especially to exist for > 2 years?

        Besides testing, i got bit trying to dev some code that i intended to override existing subclass factory/template methods. Not an esoteric usecase.

        Show
        marko.m added a comment - I think this is same issue that is responsible for the strange behavior i posted to the groovy user list? http://groovy.329449.n5.nabble.com/should-this-method-override-work-td5566431.html This feel like serious issue, especially to exist for > 2 years? Besides testing, i got bit trying to dev some code that i intended to override existing subclass factory/template methods. Not an esoteric usecase.
        blackdrag blackdrag made changes -
        Component/s groovy-runtime [ 16250 ]
        Hide
        SUKRIT KHERA added a comment -

        Just ran into this issue.

        +1 To provide a fix for this issue.

        Show
        SUKRIT KHERA added a comment - Just ran into this issue. +1 To provide a fix for this issue.

          People

          • Assignee:
            Unassigned
            Reporter:
            Alex McManus
          • Votes:
            6 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated: