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

Spring DI layer ingores @Scope#scopeName #12233

Closed
geoand opened this issue Sep 21, 2020 · 10 comments · Fixed by #12234
Closed

Spring DI layer ingores @Scope#scopeName #12233

geoand opened this issue Sep 21, 2020 · 10 comments · Fixed by #12234
Assignees
Labels
area/spring Issues relating to the Spring integration kind/bug Something isn't working
Milestone

Comments

@geoand
Copy link
Contributor

geoand commented Sep 21, 2020

@scope#value however does work

@geoand geoand added the kind/bug Something isn't working label Sep 21, 2020
@geoand geoand self-assigned this Sep 21, 2020
@geoand geoand added the area/spring Issues relating to the Spring integration label Sep 21, 2020
geoand added a commit to geoand/quarkus that referenced this issue Sep 21, 2020
geoand added a commit that referenced this issue Sep 21, 2020
Spring' @scope#scopeName is now taken into account
@rmarting
Copy link

I have a sample application declaring beans as prototypes, but it is not working as expected in Spring framework.

A copy of the application is available in the branch feature/quarkus-edition of my repo.

My configuration class where I declared the prototype bean is:

@Configuration
public class KafkaConfig {
    @Bean
    // Not provided by Quarkus Spring DI Extension
    //@Scope(scopeName = ConfigurableBeanFactory.SCOPE_PROTOTYPE)
    @Scope("prototype")
    public Producer<String, Message> createProducer() { /* Some stuff */ }
}

That been it used in a Service class, declared as:

@Service
public class MessageService {

    // Not implemented in Spring DI
    //private ObjectFactory<Producer<String, Message>> producer;
    @Autowired
    Producer<String, Message> producer;

This class has a method to use the prototype instance each time is invoked. The first time invoked works however when I invoke a second time then the producer instance throws the following exception:

Caused by: java.lang.IllegalStateException: Cannot perform operation after producer has been closed
	at org.apache.kafka.clients.producer.KafkaProducer.throwIfProducerClosed(KafkaProducer.java:869)
	at org.apache.kafka.clients.producer.KafkaProducer.doSend(KafkaProducer.java:878)
	at org.apache.kafka.clients.producer.KafkaProducer.send(KafkaProducer.java:862)
	at org.apache.kafka.clients.producer.KafkaProducer.send(KafkaProducer.java:750)
	at com.rmarting.kafka.service.MessageService.publishRawMessage(MessageService.java:127)
	at com.rmarting.kafka.service.MessageService.publishSync(MessageService.java:84)
	at com.rmarting.kafka.api.ProducerController.sendToTopic(ProducerController.java:52)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)

So the service class is using a singleton instance of the producer instead of a new one in each execution.

I know that a good practice is use singleton instances, however this approach is valid in Spring and it is common to see in many projects. Why it is not working in my case?

@geoand
Copy link
Contributor Author

geoand commented Sep 22, 2020

@rmarting after looking into this, my conclusion is that the Quarkus Spring DI behavior is correct.
To fix the problem with your application you need to make MessageService a @Scope("prototype") as well - because currently it's a singleton and thus when injected into the controllers the same instance of it is injected and therefore @Scope("prototype") on Producer<String, Message> is having no effect

@rmarting
Copy link

Sorry @geoand but it seems that your comments are wrong. I annotated the service class with @Scope("prototype") and the issue persists. The service, consumer and producer beans are singletons all of them.

The original code running in Spring does not force me to annotate the service as prototype, basically because I use an instance of ObjectFactory (as it is described here)

private ObjectFactory<Producer<String, Message>> producer;

That implementation works successfully in Spring framework. So Quarkus Extension for Spring DI is not covering successfully this scope ... this sample code demostrate it. (Maybe I am wrong in something else in the Quarkus implementation of my project) but the original code from Spring works.

Ideas?

@geoand
Copy link
Contributor Author

geoand commented Sep 22, 2020

Sorry @geoand but it seems that your comments are wrong. I annotated the service class with @Scope("prototype") and the issue persists. The service, consumer and producer beans are singletons all of them.

I tried this as well, and when I did, a new MessageService was being injected in each injection point (the 2 controllers).

The original code running in Spring does not force me to annotate the service as prototype, basically because I use an instance of ObjectFactory (as it is described here)

private ObjectFactory<Producer<String, Message>> producer;

That implementation works successfully in Spring framework. So Quarkus Extension for Spring DI is not covering successfully this scope ... this sample code demostrate it. (Maybe I am wrong in something else in the Quarkus implementation of my project) but the original code from Spring works.

I am assuming that ObjectFactory obtains a prototype. We could add this, but so far I don't recall of people asking for it, so we haven't done it.

Ideas?

See my first comment.

@rmarting
Copy link

But did you tried to invoke twice the REST endpoint?

Because the first time works successfully, but when you invoke again the same endpoint, then the exception appears. The problem here is the Producer or Consumer instance is the same, and when you tried to invoke to send or poll messages from Apache Kafka then some internal objects are closed, because the service is reusing the previous instance closed after the first method invokation.

@geoand
Copy link
Contributor Author

geoand commented Sep 22, 2020

Then it sounds you want the Controller to be request scoped, no?

@rmarting
Copy link

Annotated all the componentes (RestControllers, Service and Beans) as prototypes then the application works. (Code pushed into the branch).

However I am not sure it is the best approach to migrate this case from Spring to Quarkus Spring DI Extension.

I am trying to identify the best approach to migrate the original Spring code in this application using the Quarkus Extensions for Spring. Basically I would like to identify if I can use the original source code without refactor to CDI. If I have to add more extra definitions in my classes, then the best approach should be to refactor to CDI instead of to mantain the original Spring code.

I found this issue with prototypes and I only want to understand the best steps to migrate without a refactor. I refactored in other branch to use CDI annotations and there it is working successfully.

@geoand
Copy link
Contributor Author

geoand commented Sep 22, 2020

I understand your point and it makes perfect sense.

To be honest, it seems to me that the pure Spring application worked by accident, or by some obsure Spring feature that switched the scope of beans.
The way I see it, changing the scopes under the covers is really dangerous.
And FWIW, in all the Spring applications I developed over the years, I never once used prototype scope :)

@rmarting
Copy link

The Spring framwork best practices say that: use Singletons ... however my field experience showed me that from time to time a prototype object is needed and this is the reason I developed this use case (regarding other things I tried to verify).

So, finally if Quarkus Spring DI could manage successfully the prototype scope, maybe it is better in that case to refactor to Quarkus CDI and avoid side effects in the final code. Does it make sense?

Where we could describe this approach? Blog? Quarkus Guide?

@geoand
Copy link
Contributor Author

geoand commented Sep 22, 2020

The Spring framwork best practices say that: use Singletons ... however my field experience showed me that from time to time a prototype object is needed and this is the reason I developed this use case (regarding other things I tried to verify).

So, finally if Quarkus Spring DI could manage successfully the prototype scope, maybe it is better in that case to refactor to Quarkus CDI and avoid side effects in the final code. Does it make sense?

I would say that there is no hard rule, but that does make sense - if you are doing obscure DI stuff (which you are not even sure would work in Spring proper), then it probably makes sense to migrate to CDI

Where we could describe this approach? Blog? Quarkus Guide?

I'll let @jclingan comment on this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/spring Issues relating to the Spring integration kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants