----- Original Message ----- > From: "Vojtech Szocs" <[email protected]> > To: "Juan Hernandez" <[email protected]> > Cc: "engine-devel" <[email protected]> > 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" <[email protected]> > To: "Vojtech Szocs" <[email protected]> > Cc: "engine-devel" <[email protected]> > 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 _______________________________________________ Engine-devel mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-devel
