Hi,
This is correct. 'pluginDefinitions' HashMap state is not
expected to be modified, at least for now (e.g. using Collections.unmodifiableMap).
forgot about changing JsonNode values themselves, within the HashMap :) we should treat
them as immutable for now.
Vojtech
----- Original Message -----
From: "Vojtech Szocs" <vszocs(a)redhat.com>
To: "Chris Frantz" <Chris.Frantz(a)hp.com>
Cc: "engine-devel" <engine-devel(a)ovirt.org>
Sent: Tuesday, September 11, 2012 2:38:47 PM
Subject: Re: [Engine-devel] UI Plugins configuration
Hi Chris,
sorry for my late response, I'm working on PoC patch rev.5 this week, and was thinking
about your ideas, and came up with some modifications. Please let me know what you think.
Regarding 'configFile' property in plugin descriptor, maybe we could remove it, in
favor of introducing configuration file naming convention
'<descriptorFileName>-config.json':
(+) there would be a standard plugin configuration file naming convention [making it
simpler for plugin end users]
(+) Engine (WebAdmin servlet) wouldn't have to parse plugin descriptor just to
determine configuration file name
This way, reloading plugin descriptors/configuration could work like this:
a, iterate through plugin descriptor [*.json] files in $DATADIR/ui-plugins/ directory,
remember 'dateLastModified' value
b, iterate through plugin configuration [<descriptorFileName>-config.json] files in
$CONFIGDIR/ui-plugins/ directory, remember 'dateLastModified' value
c, for each plugin descriptor:
c1, if (descriptorFileNameNotInCache || currentDescriptorDateLastModified >
cachedDescriptorDateLastModified) { goto c3; }
c2, else if (currentConfigDateLastModified > cachedConfigDateLastModified) { goto
c4; }
c3, reload (parse/validate/cache) plugin descriptor
c4, reload plugin configuration
This is based on following assumptions:
1. plugin descriptor [$DATADIR/ui-plugins/<descriptorFileName>.json] is NOT intended
to be modified by end users [packaging system handles this]
-> we don't expect plugin descriptor data to change too often
2. plugin configuration [$CONFIGDIR/ui-plugins/<descriptorFileName>-config.json] is
intended to be modified by end users
-> we expect plugin configuration data to change more frequently
This is why I proposed the above mentioned reloading process, in favor of reloading
everything when any timestamp (descriptor/configuration) changes to a newer value. Let me
know what you think.
As far as synchronization goes, the current code's
getPluginDefinitions() loads all of the plugin descriptors and configurations into a new
HashMap and then assigns that hashmap to WebadminDynamicHostingServlet.pluginDefinitions.
As long as the JsonNode trees in pluginDefinitions aren't modified after being put
into pluginDefinitions, I don't see a synchronization issue. Successive calls to get
data from the pluginDefinitions hashmap may see different data, but they'll never see
inconsistent data (e.g. a partially constructed plugin descriptor).
This is correct. 'pluginDefinitions' HashMap state is not expected to be modified,
at least for now (e.g. using Collections.unmodifiableMap). But consider following use-case
when invoking WebadminDynamicHostingServlet:
[thread #1] nothing needs to be reloaded, writes plugin meta-data into WebAdmin HTML page,
writes data for 'plugin1' (1/2 plugins)
[thread #2] both 'plugin1' and 'plugin2' descriptor/configuration has
changed, reloads them, writes plugin meta-data into WebAdmin HTML page (both plugins),
request finished
[thread #1] writes data for 'plugin2', request finished
As you can see, thread #1 will produce plugin meta-data with 'old' data for
'plugin1', and 'new' data for 'plugin2'. I guess a more
appropriate behavior would be that thread #1 will produce 'old' data for both
plugins, since nothing new was detected at time when thread #1 checked for changes
("nothing needs to be reloaded"). What do you think?
However, if we want to add calls to modify the pluginDefinitions at
runtime (such as a element in the GUI that disables certain plugins), then we will need
synchronization. We'll also have to concern ourselves with whether any such
modifications need to be saved back to the files on disk.
I'd like to avoid such synchronization. Plugin descriptor/configuration data, after
being loaded, should be treated as immutable for the given timestamp.
For example, if the user wants to disable pluginX across multiple WebAdmin runs, he can do
so via WebAdmin GUI, with "disable pluginX" information persisted in HTML5 local
storage (through WebAdmin's ClientStorage abstraction, using HTML5 if possible,
falling back to cookies when not available). What do you think?
Regarding GUI -> plugin file changes, I'd also like to avoid it, we could use above
mentioned HTML5 local storage for any changes on top of descriptor/configuration data.
Cheers,
Vojtech
----- Original Message -----
From: "Chris Frantz" <Chris.Frantz(a)hp.com>
To: "Vojtech Szocs" <vszocs(a)redhat.com>
Cc: "engine-devel" <engine-devel(a)ovirt.org>
Sent: Wednesday, September 5, 2012 8:52:28 PM
Subject: RE: UI Plugins configuration
Vojtech,
Please go ahead and include my ideas and/or code into your next patch.
With regards to the FIXME, I think using the 'date last modified' is a good idea.
However, the plugin descriptor file can also reference the plugin config file (via the
configFile property). I imagine that adding, removing or reconfiguring plugins will be a
relatively rare event, so maybe there is a simpler method:
Remember the newest timestamp of ($DATADIR/ui-plugins/*, $CONFIGDIR/ui-plugins/*) and
reload the plugin descriptors and configurations if the newest timestamp changes.
As far as synchronization goes, the current code's getPluginDefinitions() loads all of
the plugin descriptors and configurations into a new HashMap and then assigns that hashmap
to WebadminDynamicHostingServlet.pluginDefinitions. As long as the JsonNode trees in
pluginDefinitions aren't modified after being put into pluginDefinitions, I don't
see a synchronization issue. Successive calls to get data from the pluginDefinitions
hashmap may see different data, but they'll never see inconsistent data (e.g. a
partially constructed plugin descriptor).
However, if we want to add calls to modify the pluginDefinitions at runtime (such as a
element in the GUI that disables certain plugins), then we will need synchronization.
We'll also have to concern ourselves with whether any such modifications need to be
saved back to the files on disk.
--Chris
-----Original Message-----
From: Vojtech Szocs [mailto:vszocs@redhat.com]
Sent: Tuesday, September 04, 2012 7:46 AM
To: Frantz, Chris
Cc: engine-devel
Subject: Re: UI Plugins configuration
Thank you Chris :)
I'd like to incorporate your ideas into upcoming PoC patch, which will be focused on
server-side components of UI plugin infrastructure:
1) servlet that serves plugin-related static resources (plugin host page, actual plugin
code, any 3rd party JavaScript libraries, CSS, etc.) from local filesystem
2) class responsible for parsing/validating/caching plugin descriptor information from
local filesystem
Regarding the comment in WebadminDynamicHostingServlet that says "FIXME: do we load
this everytime, or just once?": maybe we could use the same approach as in servlet
containers, which use 'date last modified' attribute of JSP files to trigger
recompilation of corresponding servlets. For example, each time WebAdmin host page gets
requested through WebadminDynamicHostingServlet, we could iterate over plugin descriptor
files (in /usr/share/ovirt-engine/ui-plugins), looking at 'date last modified'
attribute, parsing/validating plugin meta-data, and caching it using pluginName +
dateLastModified (compound cache key). We also need to synchronize the access to plugin
meta-data. What do you think?
After the upcoming PoC patch (focused on server-side components) gets released, UI plugin
infrastructure should be pretty much stable and we can focus on adding particular
features, such as:
* adding custom sub-tabs
* adding custom context-menu items
* making REST API calls through plugin API
* investigating cross-origin plugin scenario (plugin gets loaded from page sitting on
different origin than Engine JBoss instance)
Let me know what you think.
Cheers,
Vojtech
----- Original Message -----
From: "Chris Frantz" <Chris.Frantz(a)hp.com>
To: "Vojtech Szocs" <vszocs(a)redhat.com>
Cc: "engine-devel" <engine-devel(a)ovirt.org>
Sent: Thursday, August 30, 2012 10:03:02 PM
Subject: RE: UI Plugins configuration
Vojtech,
Here is my patch against WIP-UI-Plugins-PoC-revision-4. I’ve also included 2 dummy plugins
in sample.tar.gz.
--Chris
From: engine-devel-bounces(a)ovirt.org [mailto:engine-devel-bounces@ovirt.org] On Behalf Of
Frantz, Chris
Sent: Thursday, August 30, 2012 1:06 PM
To: Vojtech Szocs
Cc: engine-devel
Subject: Re: [Engine-devel] UI Plugins configuration
Vojtech,
I agree with your formalized names:
Plugin Descriptor is the JSON file containing plugin meta-data. The plugin descriptor may
also contain the default configuration data. It is located in $DATADIR/ui-plugins.
Plugin Configuration is the JSON file containing optional plugin configuration info. It is
located in $CONFIGDIR/ui-plugins (unless the Plugin Descriptor contains an absolute path).
Plugin Definition is the JavaScript object used by WebAdmin. In the current
implementation, the Plugin Definition contains both the Plugin Descriptor and the Plugin
Configuraion.
Plugin Source Page is the HTML page used to invoke the plugin code and shall be referenced
by the plugin descriptor’s “url” attribute.
I’ve implemented the config merging you’ve suggested: the structure in configFile gets
merged with the structure of “config”, with the data in configFile winning in the case of
duplicate key names.
BTW, the patch is against ovirt-engine + 0001-WIP-UI-Plugins-PoC-revision-2.
Let me know what you think,
--Chris
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel