Enhancing @Transactional service gives groovy.lang.MissingMethodException

[kszymko]

Grails 3.3.8 / JDK1.8.0_192 / Win OS / grails-melody-plugin 1.74.0

When adding new Service class:

package grails.melody.plugin
import grails.gorm.transactions.Transactional

@Transactional
class DomainFindService {
    def findByName(Class domain, String name) {
        log.debug 'findByName - name [{}]', name
        Object instance = domain.findByName(name)
        log.debug 'findByName - [{}]', instance
        return instance
    }
}

and extending test from app-integration-tests like this

package grails.melody.plugin

import grails.testing.mixin.integration.Integration
import grails.transaction.Rollback

import org.springframework.transaction.annotation.Transactional

import spock.lang.Specification

@Integration
class GrailsTransactionSpec extends Specification {

    DomainFindService domainFindService

    @Transactional
    @Rollback
    def 'saving an domain instance should not throw IllegalStateException'() {
        when:
            new SampleDomain(name: 'ABC').save(flush:true)

        then:
            notThrown IllegalStateException
            SampleDomain.count() == 1

        when:
            Object instance = domainFindService.findByName(SampleDomain, 'ABC')

        then:
            notThrown IllegalStateException
            instance != null
    }

}

gives:

    groovy.lang.MissingMethodException: No signature of method: grails.melody.plugin.DomainFindService.$tt__findByName() is applicable for argument types: (java.lang.Class, java.lang.String, org.springframework.transaction.support.DefaultTransactionStatus) values: [class grails.melody.plugin.SampleDomain, ABC, org.springframework.transaction.support.DefaultTransactionStatus@46cab43e]
        at grails.melody.plugin.MelodyInterceptorEnhancer.enhance_closure1$_closure2(MelodyInterceptorEnhancer.groovy:71)

Looks like first argument's type Class makes all the troubles.
When switched to Object works OK and metaMethod is found in MelodyInterceptorEnhancer:

def metaMethod = delegate.metaClass.getMetaMethod(name, args)

Does it look like a bug to you or is it documented somewhere
(NOT to use certain method signatures to be properly enhanced by Melody)?

Chris

[sergiomichels]

Hi Chris, we don't have documentation for supported method signatures but in my opinion we should try to support any signature.

Just by curiosity, does your service works with this signature if you're not using Melody plugin?

[kszymko]

Hi Sergio,

yes it works fine, when Melody plugin is OFF.

I found this problem when I was refactoring legacy code and replacing method's parameters types from def to concrete types. When I switched def to Class my integration tests (IT) stopped working.

When I turned OFF Melody IT worked fine even after refactoring.

[sergiomichels]

So I was checking this behavior and looks like an issue (or maybe expected behavior?) with meta class.

Currently we rely on metaClass.getMetaMethod(name, args) to be able to invoke service methods and properly collect statistics. With your signature the generated method $tt__findByName() fails to be found, I also tried looping in list of all methods available and it's not there...

Without Grails Melody plugin your service call will work because there's no metaClass.invokeMethod attached and Groovy will use default method resolution to execute it.

Maybe we could use Spring AOP instead of meta class , but this needs more time to investigate. For now unfortunately you'll have to change your method signature.

[virtualdogbert]

I too have just run into this error working on bug, found when upgrading to Grails 3.3.9. While I like Melody, for taking a look at performance, I've found, that leaving it on all the time can run into issues. It also have a tendency to breaking the debugger, so I often run with it off when running locally.

Another idea would be to look into compile-time metaprogramming, vs the runtime variety. Possibly using a global AST transform. This would probably be a big undertaking though, as ASTs are a bit more complex than the runtime metaprogramming.

[virtualdogbert]

So after a little more investigation, this only seems to happen, when I have an @transactional, on a method, that has a parameter of type Class. I don't know it that will help any though.

[virtualdogbert]

Ok, I started playing with it and converting to an AST might not be that hard. The only difficulty is getting the configuration to the AST, when in my experience, I've only been able to pull off by setting a system property in a custom compiler script. So from the MelodyInterceptorEnhancer it looks like that just intercepts every call to methods in services, and adds a call to SPRING_COUNTER.bindContextIncludingCpu(requestName), unless I'm missing something. The only question I have is by default it seems that SPRING_COUNTER.isDisplayed() is false, what do I have to set to make that true?

[sergiomichels]

@virtualdogbert Can you share a branch with your changes? Then I can take a look in your issue.

[virtualdogbert]

@sergiomichels Ok I'll gather up what I have tonight, push it to github, and send you links. Like I said the big thing I was missing from a testing stand point was how to get SPRING_COUNTER.isDisplayed() to be set true.

The only reason I hit you up on slack was because it had been over a month since I posted this, now it's two. Although I totally understand about being busy, I have to get back to some of my side projects as well...

[sergiomichels]

@virtualdogbert There's a bug in MelodyInterceptorEnhancer, not sure if it was a change in Melody because plugin code is in this way since version for Grails 2.

Plugin mimics Melody's MonitoringSpringInterceptor and you can see how counter is initialized in this class:

public MonitoringSpringInterceptor() {
		super();
		SPRING_COUNTER.setDisplayed(!COUNTER_HIDDEN);
		SPRING_COUNTER.setUsed(true);
		LOG.debug("spring interceptor initialized");
	}

[virtualdogbert]

@sergiomichels So I put my idea for replacing the runtime programming with an AST Transform here:
https://github.com/virtualdogbert/grails-melody-plugin/tree/astEnhancer

I also created a test app here:
https://github.com/virtualdogbert/testMelody

I didn't make the AST configurable, at this point, I just commented out the runtime metaprogramming in the plugin and used the AST. However I still don't know how to get the spring monitoring going, through the config, it doesn't seem to hit the class that you mention...

I can make it so that it could be a choice between the AST way and the runtime metaprogramming way similar to how I made the Enterprise Groovy Gradle plugins AST configurable, using a compiler script.

[sergiomichels]

@virtualdogbert this interceptor class is not used by grails melody, the goal of runtime metaprogramming is to have same logic, so init from interceptor should be copied to plugin bootstrap.