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

OSGi : RouteRegistryInitializer which is responsible for servlet initialization logic is not invoked in OSGi #4376

Closed
denis-anisimov opened this issue Jul 9, 2018 · 12 comments
Assignees
Milestone

Comments

@denis-anisimov
Copy link
Contributor

denis-anisimov commented Jul 9, 2018

This issue is specific for Jetty in Felix OSGi container (both org.apache.felix.http.jetty and jetty osgi boot).
May be this is a question of the web server bundle in OSGi and there is nothing to do on our side.

I register the servlet manually via the bundle activator (using HttpServiceTracker and manual servlet instance creation).
And the application doesn't have any routes available even though I have class with @Route annotation.
It seems that RouteRegistryInitializer doesn't do its job.

See #4367 for the servlet registration source code.

@denis-anisimov denis-anisimov changed the title OSGi : ServletDeployer which is responsible for servlet initialization logic is not invoked in OSGi OSGi : RouteRegistryInitializer which is responsible for servlet initialization logic is not invoked in OSGi Jul 9, 2018
@denis-anisimov denis-anisimov self-assigned this Jul 10, 2018
@denis-anisimov
Copy link
Contributor Author

It looks like this is a question of the web server bundle which is used in OSGi container.
I'm using jetty inside apache felix via org.apache.felix.http.jetty.
I have a number of bundles related to a proper annotation handling in Jetty but it still doesn't work.
There is another option: use jetty osgi boot bundle. It starts Jetty as a web server and allows to deploy war bundles directly to Jetty. I wasn't able to make it work either but this way is quite specific for Jetty.
I can't even get an HTTP service in a bundle when Jetty is started via osgi boot.

@denis-anisimov
Copy link
Contributor Author

Another thing to mention is jetty websocket implementation which uses ServletContainerInitializers
has an implementation of this interface registered as a Java service (Java SPI ServiceLoader mechanism). As a result ServiceLoader RI is required to be installed on OSGi container (in this case) but I still can't get it working: it seems no-one calls the ServletContainerInitializers.

@denis-anisimov
Copy link
Contributor Author

denis-anisimov commented Jul 12, 2018

I don't know whether this is a common issue with OSGi containers and web server bundles but
in my case it's Jetty bundle inside Felix.
There are two ways to use Jetty in Felix: org.apache.felix.http.jetty bundle and Jetty :: OSGi :: Boot bundle.

The first one doesn't care about ServiceContainerListeners at all. It doesn't load them and has no functionality to run them.
So it looks like it's not possible to use this server bundle with Flow without a lot of additional custom code.

Jetty :: OSGi :: Boot allows to have ServletContanerInitializer but there are several issues:

  • It's not enough to declare class which implements ServletContanerInitializer and has HandlesTypes annotation. jetty-annotations bundle will discover initializers only if they are registered explicitly via Java SPI (ServiceLoader) (which requires ServiceLoader RI to be installed in the OSGi container).
  • jetty-annotations works only with WebAppContext which assumes there is a war file. I was not able to make them work with plain bundle.
  • Jetty :: OSGi :: Boot doesn't provide HttpService which means we can't rely on the Vaadin client side resources registration which is done in the client activator via standard OSGi mechanism.

As a result : if you use felix jetty then you will have HttpService but no ServletContainerInitializers. If you use Jetty :: OSGi :: Boot then you should use a usual way to deploy web application via War file to Jetty bundle and there is no OSGi specific: everything should work out of the box as in plain Web container.

As a result in my specific case I have a jar bundle which register VaadinServlet and has some routes.
I'm using Jetty :: OSGi :: Boot bundle and have a custom HttpService which is implemented on top of Jetty ServletContextHandler.
The servlet is registered via ServletContextHandler.

In the result jetty boot make servlet initialization with Servlet context based on WebAppContext(this servlet context is never used in reality in any created servlet later on).
But any real (vaadin) servlet which handles requests uses another serlvet context based on ServletContextHandler and as a result there are no any routes available for this servlet context.

I made a hack which grabs this unused servlet context (WebAppContext based) and this context is used as a servlet context for any servlet which is created in my bundle.

Only this way makes the bundle work with Flow: routes are registered correctly and client resources are available.

I don't know whether this is an issue which we have to solve on our side somehow to make it work properly with OSGi containers.
I will use my hack for the tests for now.
And will keep this open. May be we will need to fix it if this is a common problem for OSGi bundles.

@denis-anisimov denis-anisimov removed their assignment Jul 12, 2018
@Sandared
Copy link

Hi Denis,

I asked for the ServletContainerInitializer on the felix mailing list and the reason this is not supported is the dynamic nature of OSGi. So in a usual ServletContainer it is valid to have a class that does initialization work on startup, but in OSGi there could be servlets that are coming and going during runtime. (At least that's my understanding)

So what I would try in your case is to write a RouteRegistryInitializer that is not a ServletContainerInitializer but an OSGi whiteboard that's listening for registered/unregistered services that have a common interface (e.g. Component)

This would look like this:

@Component
public class OSGiRouteRegistryInitializer {
  private final List<Component> routes= new CopyOnWriteArrayList<>();

  @Reference(
	cardinality = ReferenceCardinality.MULTIPLE,
	policy = ReferencePolicy.DYNAMIC)
  void addRoute(Component foo) {
	routes.add();
       // do something else with foo
  }

  void removeRoute(Component foo) {
	routes.remove(foo);
       // do something else with foo
  }
}

whereas all @routes are OSGi components like this

@Component(service=Component.class)
@Route("")
public class MyRoute extends Div {
  public HelloWorld() {
    setText("Hello world");
  }
}

Maybe this could work if it is possible to add/remove Routes dynamically to/from the RouteRegistry which might relate to #3913 .

Maybe then for each Route one would have to use another ServletContext that enables to load resources from the respective bundle, but I'm not sure about that as I'm not that deep into Servlets and stuff like that ;)

Maybe thos hints help you to realize the OSGi support in Vaadin 11.

Kind regards,
Thomas

@denis-anisimov
Copy link
Contributor Author

denis-anisimov commented Jul 16, 2018

Thanks a lot for the help!

Well, so it means that OSGi doesn't really support Servlet 3.0 specification since ServletContainerInitializer is a part of it.

The problem is that Flow relies on this specification in many many places.
Routes are just a part of this issue.
There are other annotations that we need to be scanned during the bootstrap phase.
So we really would like to have ServletContainerInitializer working at least somehow.
The requirement to have @Component(service=Component.class) in addition to @Routeis quite heavy requirement from my opinion because:

  • this is pure OSGi solution which is not needed for non-OSGi world. As a result the same application without those annotations will work in non-OSGi servlet container and won't work in OSGi .
  • this is just extra annotation which makes it inconvenient to use. As I mentioned there are other annotations which we also scans during initialization phase and the same should be done for all of them. I think it's really inconvenient especially because of the previous item ( in non-OSGi it's not needed).

So may be we need to do something specific for OSGi but I think it should be something on our side (additional logic specific for OSGi implemented in Flow).

Thank you for your help one more time!

@Sandared
Copy link

I have to thank YOU for your effort on this. I know that OSGi can be tough some times, as it requires one to rethink old patterns , which I had to learn the hard way ;)

Regarding the @Component annotation:

  • Yes it is OSGi specific, but the alternative would be to write the Declarative Services XML manually, which is possible, but in my opinion even worse than the annotation
  • IF one is using Vaadin in OSGi context, then a @Component annotatin would be sensible, as you probably want to use your Route as a service that can reference other services via @Reference, which is not possible if you didn't annotate the class with @Component
  • The annotation has no side effect in non-OSGi environment, as it is an annotation that is processed at compile-time. The only drawback would be that in a non-OSGi environment the class would still have a dependency to the OSGi API package that contains the annotations.

Regarding the "additional logic specific for OSGi implemented in Flow":
Yes it is needed, but in OSGi you often achieve such things by creating a new bundle that serves as kind of bridge between your old code and the new one... or between non-OSGi code and OSGi code. An example would be the old Vaadin8 OSGi integration, that was in a separate bundle that only had to be present at runtime if the application should run in an OSGi environment. This kind of logic is usually realized by some sort of Whiteboard Pattern as in my comment before.

Regarding "Flow relies on this specification in many many places":

  • Could you point me to those places? Maybe those could then be solved with a similar mechanism as the RouteRegistryInitializer which would require not that much code.
  • If it really gets complicated, then maybe people like @cziegeler could give a hint on how this could be solved in OSGi, as he always so kindly answers my (somteimes stupid) questions about felix and jetty ;)

I really hope that this helps, as I (and I think also many others) would love to use Vaadin 11 in my OSGi applications.

Kind regards,
Thomas

@denis-anisimov
Copy link
Contributor Author

Could you point me to those places? Maybe those could then be solved with a similar mechanism as the RouteRegistryInitializer which would require not that much code.

They are quite similar yes, but anyway..... It becomes hard to be aware of all of these things.
At the moment we have several ServletContainerInitializer implementations:

  • AnnotationValidator. It cares about Viewport , BodySize , Inline , Theme , Push annotations. Bu this is just a validator.
  • ErrorNavigationTargetInitializer cares about HasErrorParameter annotation and has logic specific for that.
  • ServletVerifier this is just a verifier for servlet 3.1 specification.
  • RouteRegistryInitializer cares about Route and RouteAlias . That's already known.

I'm afraid that there will be also issues with Push since it relies also on ServletContainerInitializer : there is at least ContainerInitializer in atmosphere bundle.

@Sandared
Copy link

Sandared commented Jul 16, 2018

I had a look at those classes and maybe you could solve the issue by replacing them with a BundleTracker that (in an OSGi environment) does the same job as the ServletContainerInitializers do in a normal environment.

This would look something like this:

// This class is just needed to start and stop the BundleTracker in OSGi
@Component
public class MyExtender {
  private BundleTracker tracker;

  @Activate
  private void activate(BundleContext context){
    tracker = new VaadinBundleTracker(context);
    tracker.open();
  }

 @Deactivate
  private void deactivate(){
    tracker.close();
    tracker = null;
  }
}

public class VaadinBundleTracker extends BundleTracker{
  private Executor executor = Executors.newSingleThreadExecutor();

  public VaadinBundleTracker(BundleContext context){
    // Bundle.ACTIVE makes this BundleTracker only track Bundles that reach or have already reached the ACTIVE state
    super(context, Bundle.ACTIVE, null);
  }

  @Override
  public Object addingBundle(Bundle bundle, BundleEvent event){
    // This header must be added to all bundles that have a class with @Route or any other annotation that should be processed 
    String headerName = (String)bundle.getHeaders().get("Vaadin-OSGi-Extender");
    
    if(headerName != null){
      // We use an executor as frameworkmethods should not block or take too long
      executor.execute(new Runnable(){
       
       @Override
        public void run(){
          Wiring wiring = bundle.adapt(BundleWiring.class);
          
          // get all .class resources of this bundle
          Collection<String> classes = wiring.listResources("/", "*.class", BundleWiring.LISTRESOURCES_RECURSE);

          for(String clazz : classes){
            // get the classname from the path and load the class to inspect it further
            String className = replaceAll("\\.class$", "").replace('/', '.');
            Class<?> classToInspect = bundle.loadClass(className);

           // do checks/logic for classes that implement, extend, or have been annotated with Route/RouteAlias/Viewport/etc ... 
           // that are usually done by the ServletContainerInitialize
          }
        }
      });
    } 
  }
}

For this to work, all bundles that have a Route or anything similar must add a "Vaadin-OSGi-Extender" header to their manifest, which is (in my opinion) not as intrusive as an annotation.

Nevertheless, the classes that are @Routes will probably be also @Components in an OSGi environment. Therefore, there must be some sort of bridge that connects the way Vaadin instantiates those with the way OSGi instantiates them.

Hope that helps.

Kind regards,
Thomas

@Sandared
Copy link

Just changed my code, as it seems lower letter headers are not accepted.

One could also narrow down the classes to load by only loading those in the exported/private packages of the bundle.

@pleku pleku added this to the During 2018 candidates milestone Jul 19, 2018
@pleku
Copy link
Contributor

pleku commented Oct 2, 2018

We should first test the approach recommended here #4376 (comment) (thanks @Sandared !) for the routes, and if that works, then use it for every other servlet context initializer we have. That can be left for future issues.

@pleku pleku modified the milestones: Candidates, V12 Candidates Oct 2, 2018
@pleku
Copy link
Contributor

pleku commented Oct 5, 2018

One thing - if the approach works, it should be tested that the routes from another jar work when it has the OSGi meta data. The route classes should be explicitly declared as public in OSGi context.

@denis-anisimov
Copy link
Contributor Author

There is no anything specifically done for our code base.
So nothing to change generically.

But our current tests uses jetty-osgi-boot which has an ability to run ServletContainerInitializer.
But to enable this the appropriate classes are registered: https://github.com/vaadin/flow/blob/master/flow-tests/test-root-context/src/main/resources/META-INF/services/javax.servlet.ServletContainerInitializer

So either this file needs to be removed completely or all Flow initializers should be removed.
This will disable automatic types discovering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants