[Engine-devel] Update on UI Plugins: PoC patch revision 4

[Doron Fediuck]

----- Original Message -----
> From: "Vojtech Szocs" <vszocs at redhat.com>
> To: "Juan Hernandez" <jhernand at redhat.com>
> Cc: "engine-devel" <engine-devel at ovirt.org>
> Sent: Tuesday, September 4, 2012 3:04:50 PM
> Subject: Re: [Engine-devel] Update on UI Plugins: PoC patch revision 4
> 
> Hi Juan, thanks for your comments :)
> 
> Server-side components of UI plugin infrastructure (such as
> PluginSourcePageServlet) indeed need some more work, I agree with
> your points.
> 
> I was thinking that PluginSourcePageServlet and FileServlet are quite
> similar in their purpose, serving static resources from local
> filesystem. FileServlet is intended for general use, with 'file'
> parameter configured as servlet init-param. For example, FileServlet
> could be used to serve static resources from
> /usr/share/ovirt-engine/ui-plugins:
> 
> <servlet>
>   <servlet-name>pluginResourceServlet</servlet-name>
>   <servlet-class>org.ovirt.engine.core.FileServlet</servlet-class>
>   <init-param>
>     <param-name>file</param-name>
>     <param-value>/usr/share/ovirt-engine/ui-plugins</param-value>
>   </init-param>
> </servlet>
> <servlet-mapping>
>   <servlet-name>pluginResourceServlet</servlet-name>
>   <url-pattern>/plugins/*</url-pattern>
> </servlet-mapping>
> 
> Assuming following directory convention for UI plugin descriptors and
> actual plugin resources:
> /usr/share/ovirt-engine/ui-plugins/foo.json -> Plugin descriptor
> /usr/share/ovirt-engine/ui-plugins/foo/start.html -> Plugin host page
> /usr/share/ovirt-engine/ui-plugins/foo/foo.js -> Actual plugin code
> (referenced by plugin host page)
> 
> Such servlet could be used to map
> "http(s)://<EngineHost>:8700/plugins/foo/start.html" to
> /usr/share/ovirt-engine/ui-plugins/foo/start.html
> (note that FileServlet is in root WAR context)
> 
> The purpose of PluginSourcePageServlet is very similar, but in terms
> of FileServlet, the 'file' parameter is not static (defined in
> web.xml as init-param), but depends on plugin meta-data (defined in
> foo.json) for each plugin.
> 
> PluginSourcePageServlet was meant to map
> "http(s)://<EngineHost>:8700/webadmin/webadmin/plugin/foo/start.html"
> to /custom/plugin/base/directory/start.html
> (note that PluginSourcePageServlet is in WebAdmin WAR context)
> 
> Juan - do you think we could modify/reuse FileServlet for serving UI
> plugin static resources? As mentioned above, the only difference is
> that the 'file' parameter (base directory) would be potentially
> different for each plugin. Please let me know what you think about
> it.
> 
> Thanks,
> Vojtech
> 
> 
> ----- Original Message -----
> From: "Juan Hernandez" <jhernand at redhat.com>
> To: "Vojtech Szocs" <vszocs at redhat.com>
> Cc: "engine-devel" <engine-devel at ovirt.org>
> Sent: Thursday, August 30, 2012 8:24:02 PM
> Subject: Re: [Engine-devel] Update on UI Plugins: PoC patch revision
> 4
> 
> Nice work Vojtech, just some comments about the
> PluginSourcePageServlet:
> 
> * You can avoid the hardcoded plugin code location with something
> like this:
> 
>   import org.ovirt.engine.core.utils.LocalConfig;
> 
>   File dataDir = LocalConfig.getInstance().getUsrDir();
>   File pluginCodeLocation = new File(etcDir, "ui-plugins");
> 
> That will result in /usr/share/ovirt-engine/ui-plugins or whatever
> directory is configured in the ENGINE_USR parameter in the
> /etc/sysconfig/ovirt-engine file.
> 
> * It is very important to check the sanity of the value of the
> "plugin"
> parameter, otherwise an attacker could send you a name with
> backpaths,
> and that can result in accessing an unexpected file. In this
> particular
> case you are adding the ".js" extension, so it won't probably result
> in
> accessing dangerous files, but anyhow it is a good practice. I would
> recommend to do something like this:
> 
>   String pluginName = request.getParameter("plugin");
>   if (pluginName != null || !isSane(pluginName)) {
>       ...
>   }
> 
> The "isSane" method can do something similar to the "isSane" method
> in
> the "FileServlet" class (I think you already mentioned this at some
> point), maybe even forbid slashes as well.
> 
> * When copying the plugin file to the generated page you can avoid
> the
> extra Buffered reader/writer as you are already using your own buffer
> in
> the "copyChars" method (which is very good practice).
> 
> For the output you can directly use "response.getWriter()" instead of
> "response.getOutputStream()", that is already buffered by the
> container.
> 
> On 08/30/2012 05:39 PM, Vojtech Szocs wrote:
> > 
> > 
> > Hello everyone,
> > 
> > as a follow-up to my last email on improving plugin API, here comes
> > the latest revision of UI Plugins proof-of-concept patch (please
> > find it attached).
> > 
> > This patch is focused on improving JavaScript plugin API, along
> > with important changes and improvements made to plugin
> > infrastructure ( PluginManager ). Let's walk through the changes
> > step by step.
> > 
> > 
> > 
> > Improved plugin API, taking some inspiration from jQuery
> > 
> > Following is a sample plugin code that uses new plugin API:
> > 
> > var myPlugin = pluginApi('myPlugin'); // Obtain plugin API instance
> > for 'myPlugin'
> > var myPluginConfig = myPlugin.configObject(); // Obtain
> > plugin-specific configuration
> > 
> > // Register event handler functions to be invoked by WebAdmin
> > // Note: all functions are optional, the plugin only defines
> > functions for events it wants to handle
> > myPlugin.register({
> > UiInit: function() {
> > var testUrl = 'http://www.example.com/' + myPluginConfig.foo; //
> > Assume plugin configuration has 'foo' attribute
> > myPlugin.ui.addMainTab('Custom Tab', 'custom-tab', testUrl); //
> > Invoke some operation using plugin API
> > }
> > });
> > 
> > myPlugin.ready(); // Event handler functions are registered, we are
> > now ready to get initialized (UiInit)
> > 
> > 
> > 
> > UI plugin life-cycle, enforced by plugin infrastructure
> > 
> > The PluginState enumeration lists possible states of a plugin
> > during its runtime:
> > 
> >     * DEFINED : This is the initial state for all plugins. Plugin
> >     meta-data has been read by PluginManager and the corresponding
> >     iframe element has been created for the plugin. Note that at
> >     this point, the iframe element is not attached to DOM yet.
> >     * LOADING : The iframe element for the plugin has been attached
> >     to DOM, which causes plugin host page (previously known as
> >     plugin source page) to be fetched asynchronously in the
> >     background. We are now waiting for plugin to report in as
> >     ready. In practice, due to JavaScript runtime being
> >     single-threaded, WebAdmin startup logic will continue to
> >     execute until the JavaScript runtime is "idle" (browser event
> >     loop returns), and at this point JavaScript plugin code gets
> >     invoked through the plugin host page.
> >     * READY : The plugin has indicated that it is ready for use. We
> >     assume the plugin has already registered its event handler
> >     object (object containing various event handler functions to
> >     be called by WebAdmin) at this point. We can now proceed with
> >     plugin initialization.
> >     * INITIALIZED : The plugin has been initialized by calling
> >     UiInit function on its event handler object. We can now call
> >     other event handler functions, the plugin is now initialized
> >     and in use.
> > 
> > 
> > Note on plugin initialization: the UiInit function will be called
> > just once during the lifetime of the plugin, after the plugin
> > reports in as ready AND WebAdmin enters the state that allows
> > plugins to be invoked (entering main section for logged-in users),
> > and before other event handler functions are invoked by the plugin
> > infrastructure.
> > 
> > 
> > 
> > 
> > Plugin meta-data is now passed to client using different format
> > 
> > 
> > Previously, plugin meta-data was embedded into WebAdmin host page
> > as a simple JavaScript object, like so:
> > 
> > 
> > var pluginDefinitions = { myPlugin: "<URL>", anotherPlugin: "<URL>"
> > }
> > 
> > 
> > 
> > Now, plugin meta-data is embedded into WebAdmin host page as a
> > JavaScript array, like so:
> > 
> > 
> > 
> > var pluginDefinitions = [
> > { name: "myPlugin", url: "<URL>", config: { "foo": 1, "bar":
> > "whatever" } },
> > { name: "anotherPlugin", url: "<URL>" }
> > 
> > ];
> > 
> > 
> > As you can see, pluginDefinitions is now an array of JavaScript
> > objects, with each object representing plugin meta-data. The
> > "name" and "url" attributes are mandatory (we need to check them
> > when loading plugin descriptors). "config" is the plugin
> > configuration (JSON) object, obtained by merging default plugin
> > configuration (defined in plugin descriptor) with custom plugin
> > configuration (defined in external plugin configuration file).
> > Note that the "config" attribute is optional.
> > 
> > 
> > 
> > In terms of Java classes, pluginDefinitions is mapped to
> > PluginDefinitions overlay type, and each meta-data object within
> > the array is mapped to PluginMetaData overlay type.
> > 
> > 
> > 
> > 
> > 
> > Note on using assert statements in client code: you might notice
> > that I'm using a lot of assert statements in Plugin class. This is
> > to ensure consistency and guard against corrupted state during
> > development. In GWT, assert statements work in a different way
> > than in standard Java VM. When debugging GWT application using
> > Development Mode, assert statements are checked and throw
> > assertion errors during runtime (they are displayed in Development
> > Mode console). However, when compiling GWT application to
> > JavaScript (Production Mode), assert statements are removed by GWT
> > compiler, so they don't affect the application running in
> > Production Mode.
> > 
> > 
> > 
> > Cheers,
> > Vojtech
> > 

Hi Vojtech,
Looks very interesting.
In the context of the client code, I have a suggestion that I'd like to be considered;
Will it be possible to allow re-using some of the existing dialogs in the system? In
this way, I may be able to use the pop-up dialog of edit-policy for example.
This will keep the look and feel, and allow people to reduce issues when re-using
existing code.

Thanks!
Doron