vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
616 stars 167 forks source link

SPIKE: how do Vaadin route targets need to be created to support OSGi declarative services #9107

Closed pleku closed 3 years ago

pleku commented 4 years ago

Related to #5125 figure out how can we get declarative service injections working on Vaadin route targets. It doesn't matter whether or not it is in webjars/npm mode.

stbischof commented 4 years ago

Could you please this issue a bit more? I added a comment on #5125. But not sure about the topic here.

com.vaadin.flow.server.startup.RouteTarget and com.vaadin.flow.di.Instantiator could just hold classes . That is a problem, because declarative services handles services. There can be multiple instances of the same class with different configurations(serviceProperties).

Service

@Route("")
@Theme(Lumo.class)
@PWA(name = "Project Base for Vaadin Flow", shortName = "Project Base")
@Component(service = com.vaadin.flow.component.Component.class)
@Designate(ocd = Config.class, factory = true)
public class MainView extends VerticalLayout {

    @Activate // R7 use method for R6
    Config config;

    @ObjectClassDefinition
    @interface Config {

        @AttributeDefinition(description = "The notification text.")
        String notification() default "Vaadin loves OSGi!";
    }

    public MainView() {
        Button button = new Button("Click me",
                event -> Notification.show(config.notification()));
        add(button);
    }
}

OSGi-Configurator config:

"com.example.starter.flow.better.MainView~vaadin":{
          "notification": "Vaadin loves OSGi!",
          "com.vaadin.flow.router.route":  "vaadin"
},
"com.example.starter.flow.better.MainView~osgi":{
          "notification": "OSGi loves Vaadin!",
          "com.vaadin.flow.router.route": "osgi"
}

Is the goal to let this run?

pleku commented 3 years ago

@stbischof this issue is about an investigation on how we would technically go and enable this. So this is the first step for #5125 as the target is to be able to for instance "enable using Vaadin components, like routes, as services in another WEB". So basically you could on-demand deploy new routes to a running OSGi based application.

And before that, we need to do some "ground work" like making an OSGi specific instantiator #9185.

stbischof commented 3 years ago

@Maho7791 maybe you want to have a look

https://github.com/vaadin/flow/blob/70860f9ce8fc8daf14e024209d2bed352656cbe7/flow-server/src/main/java/com/vaadin/flow/server/startup/RouteRegistryInitializer.java

https://github.com/vaadin/flow/blob/6e6e6e197d115f8ef2718c88b618ba87204d0d23/flow-server/src/main/java/com/vaadin/flow/server/startup/ApplicationRouteRegistry.java

denis-anisimov commented 3 years ago

The brief resume from me: the requested feature is overcomplicated, too error-prone and it's not worth to make it at all since there are simple workarounds for any usage example.

I can't do even easy looking simple ticket #5125. The problem is: the feature description looks clear though in fact there is no exact details how it should work. Once this description is written in details it becomes obvious that it's self-contradictory.

I will write here the observations based on simple example the ticket:

@Route("")
@Component(scope=ServiceScope.PROTOTYPE)
public class MainView extends VerticalLayout {

    @Reference
    GreeterService greeter;

    public MainView() {
        Button button = new Button("Click me",
                event -> Notification.show(greeter.greet()));
        add(button);
    }
}

The last question shows the main problem with any attempt to mix OSGi DS with Routes: MainView as a service won't be available (won't be registered) until there is no GreeterService.

So it may happen that GreeterService is not available but then it's registered, after a wile it's again deregistered. MainView will follow here to lifecycle to GreeterService lifecycle.

BUT @Route annotation is a static route: it should be registered if there is a class annotated with it.

So what to do with this simple example? There are two options:

The last option looks the correct one. The problem as I mentioned: this is self-contradictory. We are trying to mix static routes concept with dynamic OSGi DS. This is initially logically incorrect. Static routes @Route were designed to be static: they are registered regardless of any programatic conditions. Then if you want to use dynamic registration then there is a RouteRegistry.

The problem is confirmed by another fact: @Route is still used to make a navigation target regardless of OSGi. So if there is a component annotated with @Route and it's not an OSGI service anyhow then it should work in the same way as before. That's why the first option in the last items list makes sense as well: everything marked with @Route is registered, then Instantiator makes a decision whether to create an instance as an OSGi service or just instantiate it via default consturctor.

There are two options to implement @Route support:

The second option breaks totally any plain Route support: it won't be possible anymore to use existing routes which works perfectly in non-OSGi application inside OSGi. It will be necessary to declare any navigation target as a service.

The first option is incompatible with the second one: either we register everything in the route registry or not. It's not possible to distinguish which Route should be registered or not based only on suggested declaration @Route("") @Component(scope=ServiceScope.PROTOTYPE): Route annotation gives only routes related info and @Component is not available at runtime. So when a class annotated with @Route is eligible to be registered in the registry it's not possible to understand whether it represents a service or not.

We have to pass additional meta-info for every Route :

I quite dislike the last option: it makes the boilerplate code which needs to be written even more verbose and error-prone: too easy to forget to add the annotation for the DS.

The only option which looks valid for me is: consider every route as a service But this is problematic.

Everything leads again to the problem of absence ability to get or provide meta-information in OSGi. DS has no API at runtime: when you declare a service declaratively the XML file is created with DS service meta -info.

So the option (already discussed somewhere: https://github.com/vaadin/flow/issues/5017#issuecomment-465968422) : generate somehow the DS descriptor. Without going into the details:

(then we would use @Route annotation as something which means @Component(scope=ServiceScope.PROTOTYPE, service=HasElement.class) and generate DS descriptor by ourselves) .

Everything is quite complicated and error-prone.

Another solution could be use some runtime retention annotation which is automatically considered as @Component(scope=ServiceScope.PROTOTYPE, service=HasElement.class) : some "stereotype" .

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@Component(scope=ServiceScope.PROTOTYPE, service=HasElement.class)
@OSGiRoute

But that's not possible in OSGi out of the box. Again: we should write out own functionality which produces the DS XML service descriptor.

So I hope all these observations show how many problems and self-contradictions exist in this feature request. And below I will just show how easy to workaround the necessity of mix static Route and DS .

denis-anisimov commented 3 years ago

Let's start again with the example

@Route("")
@Component(scope=ServiceScope.PROTOTYPE)
public class MainView extends VerticalLayout {

    @Reference
    GreeterService greeter;

    public MainView() {
        Button button = new Button("Click me",
                event -> Notification.show(greeter.greet()));
        add(button);
    }
}

Above I've shown that there is a question which you should answer : what do you expect if there is no GreeterService yet?

Two answers:

Both options may be implemented without serious efforts. The first one has been already shown above:

Bundle bundle= FrameworkUtil.getBundle(MainView.class);
bundle.getService( bundle.getBundleContext().getServiceReference(GreeterService.class)).greet();

Only one thing you have to care: what to do when the service is not yet available. Just add a null check for the service reference and decide what to do in this case: this logic is something what you are responsible for.

The second option is also quite simple: don't use @Route annotation at all. Register a route programatically.

@Component(service=VaadinServiceInitListener.class)
public class VaadinServiceInitListener {

   public void serviceInit(ServiceInitEvent event){
        ServiceTracker<MainView, MainView>  tracker = new ServiceTracker<MainView, MainView> (){
        @Override
        public MainView addingService(
                ServiceReference<MainView> reference) {
            MainView result = super.addingService(reference);
            VaadinService service = event.getSource();
            ApplicationRouteRegistry registry  = ApplicationRouteRegistry.getInstance(service.getContext());
            RouteConfiguration config = RouteConfiguration.forRegistry(registry);
            config.setRoute("",MainView.class);
            return result;
        }
       );
   }
}

then impl in MainView stays the same:

Bundle bundle= FrameworkUtil.getBundle(MainView.class);
bundle.getService( bundle.getBundleContext().getServiceReference(GreeterService.class)).greet();

This is a not trivial but it doesn't require serious efforts as well. And what's more important: it allows to answer all the questions which one has to answer and avoid any black magic. And this demonstrate the correct usage of dynamic registration : dynamic registry is used to register dynamically routes instead of incorrect usage of static @Route registration.

It's quite easy to modify the example to support HasElement services and various routes (not only MainView).

denis-anisimov commented 3 years ago

Now let's consider more complicated example from here: https://github.com/vaadin/flow/issues/5017#issuecomment-465592999

@Route("")
@RequireEventAdmin
@EventTopics("/my/topic")
@Component 
public class MainView extends VerticalLayout implements EventHandler{

   @Activate
   private ComponentContext context;
   @Activate
   private BundleContext bc;

   @Reference
   private SomeBusinessLogicService sbls;

   @Activate
   void activate (Map<String, Object> props){...}

   @Deactivate
   void deactivate(){...}

   @Override
   public void handleEvent(...){...}
}

Yes: it's impossible at the moment to use anything like that. But it's quite easy to rewrite this so that it becomes possible and it allows to understand all the issues and avoid unknowns :

@Route("")
public class MainView extends VerticalLayout {
      private MainViewEventHandler eventHandler;

      public MainView(){
           Bundle bundle= FrameworkUtil.getBundle(MainView.class);
           bundle.getService( bundle.getBundleContext().getServiceReference(MainViewEventHandler.class));

          // do anything you want with eventHandler
      }
}

@RequireEventAdmin
@EventTopics("/my/topic")
@Component 
public class MainViewEventHandler implements EventHandler{

   @Activate
   private ComponentContext context;
   @Activate
   private BundleContext bc;

   @Reference
   private SomeBusinessLogicService sbls;

   @Activate
   void activate (Map<String, Object> props){...}

   @Deactivate
   void deactivate(){...}

   @Override
   public void handleEvent(...){...}
}

You get pure OSGi DS MainViewEventHandler which is able to do anything you want. MainView is registered regardless of any condition. Static route MainView and OSGi dynamic MainViewEventHandler are not mixed together which allows to properly handle any possible case. One problem which is solved here out of the box without any implicit black magic is wrong usage of UI component MainView and OSGi component MainView: the Vaadin lifecycle of MainView component is not really compatible with OSGi service lifecycle :

All these methods are never called inside request processing thread. It means you may not do anything UI related directly inside such methods. Everything has to be wrapped with UI::access. Such decomposition allows to easily keep it in mind. If MainView is the same component then it will be wide common mistake to get a Vaadin session lock inside any OSGi related method to do something with UI components.

You are also responsible for the OSGi component scope: it's up to you to decide which scope you want to use.

When MainView is used as OSGi component it always has to have prototype scope with a dedicated service interface. Such requirement are too strong and error-prone: there will be a lot of mistakes regarding to this boilerplate declaration. Something will be forgotten all the time and it won't be possible to understand why nothing works.

My resume is:

pleku commented 3 years ago

I agree pretty much with everything from the first comment. I don't think we should continue further in enabling Vaadin views as DS services or until we have some concrete requests on what problem we should be solving, what is the use case. It is too theoretical at the moment and there are many ways that we can achieve enable something, without really knowing if we're going to the right direction or not.

Personally I think using the existing solution(s) for registering routes dynamically should be enough. My hunch would be that instead of using @Route we would instead enable some specific features for controlling views as DS as part of the OSGi add-on. But I don't think there is any reason to continue to that (or any other direction) at the moment.

What I think we could do, is to focus on documenting the currently working solutions (second comment) for the use cases / problems we know of, and I think for OSGi DS it is mostly:

  1. How can I use a DS in my Vaadin views and how to react / handle when the service comes available or is removed by showing things to the user in the UI
  2. How can I dynamically enable adding Vaadin Views to my applications, meaning that the view might be available or not available and the UI (app shell) should reflect that

And to my understanding (thanks to the excellent detailed explanations @denis-anisimov ) both are possible to achieve even now.

Any further development and improvements would be then considered once there is a concrete use case and a problem that we should solve. At the moment the OSGi add-on for 19+ doesn't really have users, and unless we get those, it would be a waste of time to continue implementing complex features.

pleku commented 3 years ago

Considering this done for now. Any future plans to continue in #5125.

Maurice-Betzel commented 3 years ago
* It looks obvious what do we expect from here : the navigation target is registered as a service which allows to use declaratively another service GreeterService without boilerplate code.

* The initial expectation is wrong:  It's extremely easy to rewrite this example avoiding any OSGI DS _assuming_ the `GreeterService` _is already available_ . The only line which needs to be hanged is : `greeter.greet()` . It should be
Bundle bundle= FrameworkUtil.getBundle(MainView.class);
bundle.getService( bundle.getBundleContext().getServiceReference(GreeterService.class)).greet()

The OSGi way of doing things in a modular and dynamic way results in economically sustainable and robust software, trying to fight it will not result in a production ready solution.

* The code becomes a bit more complecated than just `greeter.greet()` but it also allows to get rid of other boilerplate code : `@Component` annotation and `greeter` declaration. So I cannot say that it's more complicated than original example.

* **The most important part which is completely missed in the feature description is: _what_ should happen if there is no `GreeterService` available _yet_.**

If a service is not available you use the Circuit breaker pattern Tipp: https://www.slideshare.net/ChristianSchneider3/popular-patterns-revisited-on-osgi

The last question shows the main problem with any attempt to mix OSGi DS with Routes: MainView as a service won't be available (won't be registered) until there is no GreeterService.

So it may happen that GreeterService is not available but then it's registered, after a wile it's again deregistered. MainView will follow here to lifecycle to GreeterService lifecycle.

BUT @Route annotation is a static route: it should be registered if there is a class annotated with it.

So what to do with this simple example? There are two options:

* `MainView`  navigation target is registered anyway: but then what should happen when navigation target is being requested for instantiation ? It doesn't exist as an OSGI service (yet). But there is a route. Then some component should be created. Create it using a plain way via default constructor ?

* Don't register `MainView`  as a navigation target at all until it becomes available as a service.

A missing service should result in this case in a HTTP 404, as simple as that.

The last option looks the correct one. The problem as I mentioned: this is self-contradictory. We are trying to mix static routes concept with dynamic OSGi DS. This is initially logically incorrect. Static routes @Route were designed to be static: they are registered regardless of any programatic conditions. Then if you want to use dynamic registration then there is a RouteRegistry.

The problem is confirmed by another fact: @Route is still used to make a navigation target regardless of OSGi. So if there is a component annotated with @Route and it's not an OSGI service anyhow then it should work in the same way as before. That's why the first option in the last items list makes sense as well: everything marked with @Route is registered, then Instantiator makes a decision whether to create an instance as an OSGi service or just instantiate it via default consturctor.

There are two options to implement @Route support:

* keep the existing way and register a navigation target annotated via `@Route`  in the registry

* don't register anything in the registry and track (via OSGi) OSGi services (e.g. `HasElement`  services)  which are annotated with `@Route`.

If i remeber correctly Vaadin alows dynamic route registration, or should add it, to support the comming and going of routes.

mstahv commented 3 years ago

Dynamically registered routes are indeed available nowadays, and maybe be the default mechanism to registers views/routes when using OSGi.

@stbischof, @maho7791 have been working on an alternative OSGi integration, what uses whiteboard pattern for route registration. This is probably exactly what you are after as well. Last time I saw it, it was still using bower mode, but work had started for npm support. I wonder if there is already something published that @Maurice-Betzel could look into?

stbischof commented 3 years ago

Here is a Vaadin-OSGi-Implementation and Examples to that can handle this Topic.