OSGi: Removed static VaadinResourceService access in liferay-integration, osgi-integration #11335
Conversation
… OsgiVaadinResources
swimmesberger
changed the title
…i-service-integration-fix # Conflicts: # shared/src/main/java/com/vaadin/osgi/resources/impl/VaadinResourceTrackerComponent.java
swimmesberger
changed the title
|
I have removed the usage of the old osgi "HttpService" and refactored the code to the the new osgi whiteboard pattern (introduced in OSGi R6 - which seems to be the minimum version for vaadin). With that in place the integration experience into an OSGi environment is much smoother. For the Http Whiteboard implementation I have chosen to add a ServletContextHelper Component (see R6 spec 140.2*) for all vaadin resources. This makes registering resources much easier because the ServletContextHelper takes care of loading the resources from the correct classpath and prepending the vaadin-{version} path. In the last commit I have also updated the bnd version from 3.3.0 to 3.5.0, this update is optional but the capabilities generated by 3.5.0 are more concise than 3.3.0. As before I have tried to no break any backward compatibility. I have also made some testing in our OSGi application and with this version we had by far the best integration experience. |
|
Hi @swimmesberger , I'm looking this from Liferay perspective. |
|
Hi @swimmesberger , @mstahv and @TatuLund The last two commits made this much better approach. When I made my first experiments with Liferay OSGi portlets I was looking a similar Declarative Services based approach for resources. I will still look this code level deeper as I was more consentrating to see if this works with Liferay 7.0/DXP and 7.1 GA2. I did test this with simple application which can be generetated with Maven archetype. The this is built with maven and result is copied to This application works with build from this pull request (JDK8) and with Liferay 7.0 / DXP 7.0 fixpack50, but it does not work with 7.1 GA2. There is another issue on #11156 which is blocking that, but it is easy to fix and after I did apply the fix then it was working also with 7.1. Also note here that latest version 8.6.2 with 7.1, but this version did not have that either. Summary, I will still look little bit the code and implementation at another day until I give my final pass :) . Thanks @swimmesberger this really good improvement. |
|
Liferay 7.1 simple fix is here #11360 @swimmesberger , your version do not have the following issue with Liferay 7.1 with above fix included. It is not possible to add example portlet on the page due this. Workaround with gogo shell as this seems to be issue of the starting order of the bundles. |
|
One thing I have noticed is that (the same behavior applies to the currently released vaadin version) there is the VaadinServlet (https://github.com/vaadin/framework/blob/master/server/src/main/java/com/vaadin/server/VaadinServlet.java) which contains code to handle static resources but in the OSGi world there is no monolithic VaadinServlet. The VaadinServlet only serves requests of the vaadin application itself. All the static resources are served via the http whiteboard pattern (which serves this resources via ResourceServlets) so all the code regarding serving static resources (on the fly compilation, caching header etc...) are not triggered in the OSGi world. |
|
With the changes in this pull request, vaadin portlets are correctly working with Liferay 7.2. |
There was a problem hiding this comment.
Reviewed 1 of 5 files at r1, 1 of 2 files at r2, 4 of 6 files at r5, 1 of 1 files at r6, 1 of 1 files at r7, 1 of 1 files at r8.
Reviewable status:complete! all files reviewed, all discussions resolved
|
This PR will be included in 8.9.0.alpha1 release. I would like to urge everybody using OSGi to verify that there is no regressions, and if there are some adverse effects to inform us. |
|
Hi, @mmerruko published a fix a long time ago and I've been using that fix ever since for my applications and it's working great. Would be cool if this would finally make it into a release. Currently I'm building my own Vaadin OSGi Integration with every Vaadin release I need to use. |
|
In our application we still use the workaround via a UIProvider to use OSGi DS. This pull request allows to properly provide a VaadinServlet as a OSGi Service. For directly providing a (Vaadin) UI as a OSGi DS component (without the UIProvider workaround) we would need a additional pull request that handles this case properly. The workaround is very simple therefore it's not that big of an issue and also does not require a custom build. Something along the lines: WebappServlet.java @Component(service = Servlet.class, property = {
HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_PATTERN + "=/webapp/*"
})
public class WebappServlet extends VaadinServlet {
private ServiceObjects<MyWebappUI> uiFactory;
@Reference
public void bindUIFactory(ServiceReference<UI> uiReference) {
BundleContext context = uiReference.getBundle().getBundleContext();
this.uiFactory= context.getServiceObjects(uiReference);
}
public MyWebappUI createUI() {
return this.uiFactory.getService();
}
@Override
protected DeploymentConfiguration createDeploymentConfiguration(Properties initParameters) {
initParameters.setProperty(VaadinServlet.SERVLET_PARAMETER_UI_PROVIDER, MainUIProvider.class.getName());
return super.createDeploymentConfiguration(initParameters);
}
}MainUIProvider.java public class MainUIProvider extends UIProvider {
@Override
public Class<? extends UI> getUIClass(UIClassSelectionEvent event) {
return MyWebappUI.class;
}
@Override
public UI createInstance(UICreateEvent event) {
final VaadinServletService servletService = (VaadinServletService) event.getService();
final WebappServlet webappServlet = (WebappServlet) servletService.getServlet();
return webappServlet.createUI();
}
}MyWebappUI.java @Component(scope = ServiceScope.PROTOTYPE)
public class MyWebappUI extends UI {
...
} |
|
thanks for the update. I'll have a look at the code you provide for the workaround. Regards |
|
@alexweirig note that this was just a "quick and dirty" example I would recommend using ServiceObjects/Prototype Scope instead of component factory for a more modern OSGi based approach. (see the edit of the other comment) |
|
@alexweirig Why don't you use the vaadin-liferay-integration module and deploy it to your osgi environment? It has a ui provider and is part of the official vaadin release. No custom code is required anymore. |
|
Hi @mkampmey |
|
@alexweirig Well, the liferay-integration module does not have dependencies to liferay APIs, but to the portlet-api and servlet-api that you possibly do not have. It is possible to deploy UIs as declarative services, but I think it won't work in a karaf container. |
With this pull request I removed the static access to the OsgiVaadinResources class which prevents all kind of issues in an OSGi/liferay environment. I did this by making the VaadinResourceService a proper OSGi service which is accessed only in a dynamic way.
I tried to keep the change backward compatible by also moving the OsgiVaadinResources class to the dynamic model of accessing the VaadinResourceService internally. I made the OsgiVaadinResources class deprecated with the note that all users of this class should move to the dynamic model by referencing the VaadinResourceService via OSGi services.
This pull request is currently open for discussion and should be tested by the different stakeholders (I have no liferay instance to test the changes - only plain OSGi containers).
Correlates strongly with the change I have made in #11334 if this pull request gets merged the change in #11334 is obsolete because it changes the way the VaadinResourceTrackerComponent work internally.
Maybe @Maurice-Betzel, @ctliv, @geletejefazo18 or someone else who works with vaadin in an OSGi/liferay environment can give some input about this change.
Fixes #10220
This change is