(resending since I got a failure from xwiki's mail content filter )
---------- Forwarded message ----------
From: Vincent Massol <vincent(a)massol.net>
Date: Sun, Jul 26, 2009 at 11:33 AM
Subject: [Wiki Importer] Design questions
To: XWiki Developers <devs(a)xwiki.org>
Hi Arun,
I've just started reviewing your module. I'll send questions as they
come. Here are some to start with:
1) Why do you need the Wiki interface? Since all implementations are
wiki-specific and since it doesn't contain any useful method I don't
see why it's needed.
2) Why do you need 2 methods in WikiImporter interface? I would have
imagine only a single method:
WikiImporter.import(...)
3) Why do you pass a byte array instead of a stream? If the import is
large a byte array will result in an OOM error.
4) WikiImporterVelocityContextInitializer has invalid javadoc. In
addition instead of exposing each importer you could expose only a
single importer factory and pass an import syntax to it. This will
reduce the coupling and not expose the implementations to the client
side (very important). Something like:
WikiImporterFactory.createImporter(ImportSyntax).
5) This should never be written (in DefaultImportWikiParser &
DefaultImportWikiRenderer for ex):
//Intialize Embeddable Component Manager
EmbeddableComponentManager ecm = new EmbeddableComponentManager();
ecm.initialize(this.getClass().getClassLoader());
The component manager must alway be passed as a dependency.
6) You shouldn't use @Component("default") but instead @Component
7) You should never use new
File(System.getProperty("java.io.tmpdir"). Instead you should use the
existing API located in
Container.getApplicationContext().getTemporaryDirectory().
I have some more comments but since they are linked to the above let's
agree first on the points above before going further.
Thanks
-Vincent