On Sun, Jul 26, 2009 at 20:18, Arun Reddy<[email protected]> wrote:
> 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.
Would be even better than each importer be a component with syntax as
role hint exactly like for parsers.
>
>
>>
>> 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 ) ?
Yes.
@Requirement
private ComponenetManager componenetManager;
>
>
>> 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
>
--
Thomas Mortagne
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs
