Started from https://github.com/spring-projects/spring-retry/issues/238
In org.springframework.cglib.proxy.Enhancer.getMethods():
CollectionUtils.filter(methods, new RejectModifierPredicate(Constants.ACC_FINAL));
If the method is final it is SILENTLY rejected from being proxied.
Shouldn't we warn someone who is implementing a proxy on top of a final class ?
I believe it is a contract break of making the proxy and should throw a NotProxiableElementException("The method or field is final.")
.
This issue is particularly severe in Kotlin where final
is the default.
This example is using Spring-Retry to generate the Proxy and demonstrate the issue.
import org.junit.Assert
import org.junit.jupiter.api.Test
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.boot.test.context.SpringBootTest
import org.springframework.retry.annotation.EnableRetry
import org.springframework.retry.annotation.Retryable
import org.springframework.stereotype.Component
private const val COUNT=1000
@SpringBootTest
@EnableRetry
class SpringOpenIssueTest {
@Autowired
private lateinit var bean: ChildBean
@Test
fun `proxies should work in kotlin with all-open plugin`(){
bean.assertOkayFromTheInside()
Assert.assertEquals(COUNT, bean.openCount)
Assert.assertEquals(COUNT, bean.closedCount) //java.lang.AssertionError: expected:<1000> but was:<0>
}
}
@Component //This class is automatically opened by all-open plugin
class ChildBean : AbstractBean(){
@Retryable
fun assertOkayFromTheInside(){
Assert.assertEquals(COUNT, openCount)
Assert.assertEquals(COUNT, closedCount)
}
}
//This class is open because abstract
abstract class AbstractBean{
var closedCount = COUNT //This field is not automatically opened
open var openCount = COUNT
}
Comment From: mdeinum
Generating a more clear warning would be nice as it is also a cause for weird behaviour (see https://deinum.biz/2020-07-03-Autowired-Field-Null/). It doesn't only apply to final
methods but also private
and default modifier methods leading to this.
I have been answering too many questions on StackOverflow on this kind of issues, so a nice warning would probably trigger people to watch closer.