micronaut-projects / micronaut-core

Micronaut Application Framework
http://micronaut.io
Apache License 2.0
6.09k stars 1.07k forks source link

@RequestScope from @Factory are not actually request-scoped #6664

Open davidhiendl opened 2 years ago

davidhiendl commented 2 years ago

Expected Behavior

A @RequestScope bean created from a @Factory (see example below) is not actually request scoped and ignores the factory. Every time the object is injected a new instance is created (tested with endpoints and request filters).

It does however work correctly when the @RequestScope annotation is moved to the SomeObject class and the factory is disabled.

Actual Behaviour

Every time the object is injected a new instance is created (tested with endpoints and request filters).

Steps To Reproduce

Example code to illustrate and reproduce the problem:

package test

import io.micronaut.context.annotation.Factory
import io.micronaut.http.HttpRequest
import io.micronaut.http.MutableHttpResponse
import io.micronaut.http.annotation.Controller
import io.micronaut.http.annotation.Filter
import io.micronaut.http.annotation.Get
import io.micronaut.http.filter.OncePerRequestHttpServerFilter
import io.micronaut.http.filter.ServerFilterChain
import io.micronaut.http.filter.ServerFilterPhase
import io.micronaut.runtime.http.scope.RequestScope
import io.micronaut.security.annotation.Secured
import io.micronaut.security.rules.SecurityRule
import org.reactivestreams.Publisher

@Factory
class SomeObjectFactory {
    @RequestScope
    fun create(): SomeObject {
        return SomeObject() // <--- a breakpoint here is never actually triggered, the entire method is never called
    }
}

//@RequestScope
open class SomeObject {
    var someProp: String? = null
}

@Filter(Filter.MATCH_ALL_PATTERN)
class SomeObjectFilter(
    private val someObject: SomeObject
) : OncePerRequestHttpServerFilter() {

    override fun getOrder(): Int {
        return ServerFilterPhase.SECURITY.after()
    }

    override fun doFilterOnce(request: HttpRequest<*>, chain: ServerFilterChain): Publisher<MutableHttpResponse<*>> {
        someObject.someProp = "value-from-filter"
        return chain.proceed(request)
    }

}

@Controller("/some-object-test")
@Secured(SecurityRule.IS_ANONYMOUS)
class SomeObjectTestController(
    protected val someObject: SomeObject,
) {
    @Get("/test")
    fun test() {
        assert(someObject.someProp == "value-from-filter")
    }
}

Environment Information

Example Application

No response

Version

3.2.2

davidhiendl commented 2 years ago

@graemerocher Can I help to validate the problem?

graemerocher commented 2 years ago

going to look at this soon. Thanks for the feedback

graemerocher commented 2 years ago

I can't reproduce this. See the attached example in Java.

Perhaps Kotlin getters/setters are final?

request-scope-test.zip

davidhiendl commented 2 years ago

@graemerocher I think you are right. Kotlin methods and final by default (requires open modifier to change or a compiler plugin to make them default open). When adding the open modifier to the property the factory does work.

What I don't understand is why the factory does not work but the singleton does without the open modifier. From my understanding of how the code is enhanced it should fail for both?

Edit: And having the org.jetbrains.kotlin.plugin.allopen gradle plugin installed also does not change anything.

Edit2: Apparently the allopen plugin requires a specific annotation to be registered to do anything. After creating a dummy annotation named

annotation class AllOpen()

and registering it with the compiler plugin

    allOpen {
        annotation("example.AllOpen")
    }

and applying it to the SomeObject class

@AllOpen
class SomeObject {
    var someProp: String? = null
}

the assert no longer fails.

rlconst commented 2 years ago

@davidhiendl Usually classes you want to make open already have annotations, so putting something like this into build.gradle should help either

allOpen {
  // Mark any classes with the following transactions as `open` automatically.
  annotations(
    "io.micronaut.aop.Around",
    "javax.transaction.Transactional"
  )
}
davidhiendl commented 2 years ago

@rlconst Thanks for the info. I solved it similarly by adding a custom explicit annotation as described above (my previous comment, Edit 2).

But still: The compile process should not fail silently plus its incredibly hard to debug.