Hi Vincent, On Sun, Jul 26, 2009 at 4:15 PM, Vincent Massol <[email protected]> wrote:
> (resending since I got a failure from xwiki's mail content filter ) > > ---------- Forwarded message ---------- > From: Vincent Massol <[email protected]> > Date: Sun, Jul 26, 2009 at 11:33 AM > Subject: [Wiki Importer] Design questions > To: XWiki Developers <[email protected]> > > > 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(...) > Wiki Interface is useful to access the Wiki Instance (MediaWiki , Confluence ) at runtime. The WikiImporter Interface first method processInputDump() process the Pages, Attachments and metadata and returns an object of Export Wiki Type( implements Wiki<T> Interface) which can be used to customize the import process. For Eg : Change in the Sitename, Preserve metadata and revision history. All such properties can be set at run time during import process.( From GUI - Wizard Interface) After customisation, second method createWikiPages() can be called on the same export type wiki object. which imports the pages to XWiki. > 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. Yes, that is an issue. During import process the dump is uploaded as Attachment. And the attachment content can be accessed as byte[] array only in the existing XWiki API. So the byte array is converted to ByteArrayStream and imported in the exisiting implementation. > > 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). > I liked the idea. Will make necessary changes to implement WikiImporterFactory. > > 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. Does dependency means a Requirement ( Injecting using @Requirement ) ? > 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 was not aware of existing api. I will change it. > > 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 > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs > I have created a jira issue - Improving existing Wiki Importer at http://jira.xwiki.org/jira/browse/XSANDBOX-55 I will be happy to hear more comments from your side. Thanks and Best Regards, Arun Reddy _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
