Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicitly set resolve strategy to OWNER_FIRST in BeanBuilder.invokeBeanDefiningClosure #269

Closed
tseveend opened this issue Apr 6, 2024 · 5 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@tseveend
Copy link

tseveend commented Apr 6, 2024

Grails plugin an error occur when doWithSpring is executed.

image

@rainboyan rainboyan self-assigned this Apr 7, 2024
@rainboyan
Copy link
Member

@tseveend Thanks for your report.
I created a demo here to reproduce the error, but I don't see any error, can you give some details about it?

@tseveend
Copy link
Author

tseveend commented Apr 7, 2024

@rainboyan I created and attached a demo with the issue. Thank you for your quick response

demo.zip

image

@rainboyan
Copy link
Member

@tseveend Thank you for the demo.

I can confirm that the relevant code that causes the error is as follows:

protected BeanBuilder invokeBeanDefiningClosure(Closure<?> callable) {
callable.setDelegate(this);
// callable.setResolveStrategy(Closure.DELEGATE_FIRST);
callable.call();
finalizeDeferredProperties();
return this;
}

According to the Closures Delegation strategy, the default Delegation strategy is Closure.OWNER_FIRST, but in fact it doesn't work the same in Groovy 3 and Groovy 4. In Groovy 4, we should set it explicitly. Maybe this is a bug in Groovy 4.

  • Closure.OWNER_FIRST is the default strategy. If a property/method exists on the owner, then it will be called on the owner. If not, then the delegate is used.
  • Closure.DELEGATE_FIRST reverses the logic: the delegate is used first, then the owner

@rainboyan rainboyan added the type: bug A general bug label Apr 8, 2024
@rainboyan rainboyan added this to the 2023.0.0-M6 milestone Apr 8, 2024
@rainboyan
Copy link
Member

I wrote a Groovy script to reproduce the issue. I'm not sure if this is a bug from Groovy 3 or Groovy 4, but the result is different when this code is executed in the GroovyConsole, and I need to check with the Groovy developers. Also, I'll fix this in Grace.

class Plugin {

    Closure doWithSpring() { {->
            Properties properties = loadProperties()
            println properties
        }
    }

    def loadProperties() {
        Properties properties = new Properties()
        properties['foo'] = 'Foo'
        properties
    }

}

class BeanBuilder {

    BeanBuilder beans(Closure<?> callable) {
        callable.setDelegate(this)
        // Groovy 4 should set it explicitly
        callable.setResolveStrategy(Closure.OWNER_FIRST)
        callable.call()
        return this
    }

    @Override
    public Object invokeMethod(String name, Object arg) {
        println "BeanBuilder.invokeMethod($name, $arg)"
        return this
    }
}

BeanBuilder bb = new BeanBuilder()
Plugin plugin = new Plugin()

Closure c = plugin.doWithSpring()
c.setDelegate(bb)
c.setResolveStrategy(Closure.DELEGATE_FIRST)

bb.beans(c)

@rainboyan rainboyan changed the title Change resolve strategy DELEGATE_FIRST -> OWNER_FIRST Explicitly set resolve strategy to OWNER_FIRST in BeanBuilder.invokeBeanDefiningClosure Apr 8, 2024
@rainboyan rainboyan reopened this Apr 25, 2024
@rainboyan
Copy link
Member

@tseveend

I reopened this issue because some tests fail, DynamicPlugin has its own invokeMethod(), it may conflict with BeanBuilder. invokeMethod .

I recommend using Spring AutoConfiguration, it's better than Plugin.doWithSpring().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants