Ignore VaadinServiceInitListener from ServiceLoader if it's also discovered in some other way

[Legioth]

If you have a VaadinServiceInitListener class defined through META-INF/services/com.vaadin.flow.server.VaadinServiceInitListener and the same class is also e.g. a Spring @Component, then each mechanism will create one instance of that listener and it will be invoked twice which may be quite confusing.

If the same class is discovered through multiple sources, then it should only be used once, and then preferably through e.g. Spring since that mechanism also allows injections and such to work as expected. Rather than just silently ignoring the other occurrence, it might be good to log a warning since the entry in META-INF/services is quite redundant.

[netbeansuser2019]

It is no need for any annotation to happend as described in https://vaadin.com/forum/thread/17604535

[Legioth]

@netbeansuser2019 It seems like I was making an invalid assumption in the forum thread.

The situation described in this ticket is still something that can happen and may be confusing in similar situations.

[Legioth]

Rather than moving this to the Spring repo and only fixing it there, I would suggest somehow doing this on the Flow side so that service init listeners defined as CDI beans or OSGi services would also automatically be deduplicated.

[denis-anisimov]

Github has frozen my long comment and I'm not able to do anything with this.
If I have a lack then it will give me a chance to post it.
But for now: I don't see any way (except very ugly) how it can be fixed generally.

It's about getServiceInitListeners implementation in the Instantiator class. So Flow may not know how it's overridden in subclasses or any impl.
It's a question how it's implemented in every add-on.

[denis-anisimov]

This issue is not a common issue but specific for add-ons which uses its own Instantiator implementation.
This is about the way how Instantiator::getServiceInitListeners method is implemented.
And subclass of DefaultInstantiator or direct impl of Instantiator should decide how to implement it (not the superclass/default impl).

I would say that this is a bug in the users code: include the same class in META-INF/services/com.vaadin.flow.server.VaadinServiceInitListener and use DI mechanism in addition to that.
So I consider this is as enhancement.

Another thing is : ServiceLoader instantiates the classes defined in the META-INF/services/com.vaadin.flow.server.VaadinServiceInitListener regardless of our intention.
So the only thing which we can do is : filter out already instantiated classes in the Instantiator::getServiceInitListeners impl. But they will be already instantiated.

The ugly way to fix it : introduce an additional method into Instantiator which is supposed to return the listeners and then implement filtering in the Instantiator::getServiceInitListeners .
But this is ugly because Instantiator is an interface and you may override the Instantiator::getServiceInitListeners method anyway. And having two methods with the same purpose which can be overridden is a bad contract: I don't understand which one to override (as a developer).

If you see a proper way to fix it generally please suggest. I don't see such a way.

[Legioth]

I was originally thinking that the logic in VaadinService.init that actually invokes the listeners could omit the duplicates, but now I realize that there isn't any good way for it to know which instance to use even if they would have the same getClass().

I was also assuming that the CDI and OSGi integrations would have their own implementations of the same method. It seems like CDI fires the event as a CDI event and there isn't any custom support for OSGi. From that point of view, it might indeed be best to only fix this is the Spring integration.