Hi Sergiu,

Since you only committed in the trunk, should this be @since 1.9M1? The same for all the other @since tags.

Merged with 1.8 branch today.

This is good for a standalone Java program. When it comes to webapps, the lifecycle is much more complex, since a webapp can be stopped and reloaded at will, in a clustered environment it can be moved from one computer to another, or it can even be stopped from within the application itself. This code is good for the moment, but it will have to be changed as soon as we introduce lifecycle events in the observation component.

I agree :) Creating the bridge for each new context is a waste of CPU and RAM.

Since the bridge is stateless, it can be a member initialized in the constructor.

...

+ context.put(VELOCITY_CONTEXT_KEY, new OpenOfficeServerManagerVelocityBridge(oomanager, docBridge)); + } +}

Fixed.

Why not override toString()?

...

+ public String getDescription() + { + return this.stateDescription; + } + }

Fixed.

in in

...

+ * These configuration properties are defined in in XWiki's global configuration file using the prefix of + * "officeimporter". + * </p> + * + * @version $Id$ + * @since 1.8RC3 + */ +public interface OpenOfficeServerConfiguration

Fixed.

I'm not sure that checking for specific values is a good decision. Perhaps checking for non-null, non-empty and non-negative?

...

+ assertEquals(OfficeUtils.getDefaultOfficeHome().getAbsolutePath(), configuration.getHomePath()); + assertEquals(OfficeUtils.getDefaultProfileDir().getAbsolutePath(), configuration.getProfilePath()); + assertEquals(50, configuration.getMaxTasksPerProcess()); + assertEquals(30000, configuration.getTaskExecutionTimeout()); + } +}

Fixed.

Is this attachment used anywhere?

...

+<attachment> +<filename>icon.png</filename> +<filesize>8674</filesize> +<author>XWiki.Admin</author> +<date>1236277246000</date> +<version>1.0</version> +<comment></comment> +</attachment>

Yes, it's used as the openoffice server administration icon in XWikiPreferences page. This icon will be replaced by a better one (I have asked laurent).

I usually delete the tag objects from the XML file before committing.

Fixed (Removed tag objects)

...

+#if($hasAdmin) + #info($msgConfiguration) + #if($request.action && $request.action=="stop")

This is best done like this: #if("$!request.action" == 'stop')

...

+ #if(!$oomanager.stopServer()) + #error($oomanager.lastErrorMessage) + #end + #elseif($request.action && $request.action=="start") + #if(!$oomanager.startServer()) + #error($oomanager.lastErrorMessage) + #end + #elseif($request.action && $request.action=="restart")

Since you're using it several times, you could store the action in a variable at the top: #set($currentAction = "$!request.action")

Fixed (learnt about $! just now)

What's with the paragraphs? IMO they aren't needed, just write:

$msgServerPath | $serverPath

...

+<p>$msgServerPath</p>|<p>$serverPath</p> +<p>$msgServerProfile</p>|<p>$serverProfile</p> +<p>$msgServerState</p>|<p>$currentState</p>

Well, without the paragraphs the table looks bit too tight.

Better:

Setting for the OpenOffice instance controlled by the office importer plugin.

...

+# OpenOffice

+#-------------------------------------------------------------------------------------

...

+

Fixed.

Since 1.9M1?

...

+#-# [Since 1.8RC3] +#-# Path to openoffice installation. +# openoffice.homePath=/opt/openoffice.org3/ + +#-# [Since 1.8RC3] +#-# Path to openoffice execution profile.

Is this right? For me it's /home/user/.ooo3/

...

+# openoffice.profilePath=/home/user/.openoffice.org/3

This is the default path structure returned by jodconverter. I think this is valid for most Linux systems. Thanks. - Asiri