27 Jul
2009
27 Jul
'09
10:20 a.m.
On Sun, Jul 26, 2009 at 20:18, Arun Reddy<vipunreddy.n(a)gmail.com> wrote:
Yes.
@Requirement
private ComponenetManager componenetManager;
--
Thomas Mortagne
Hi Vincent,
On Sun, Jul 26, 2009 at 4:15 PM, Vincent Massol <vincent(a)massol.net> wrote:
Would be even better than each importer be a component with syntax as
role hint exactly like for parsers.
(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(...)
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
devs(a)xwiki.org
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
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs