[ http://issues.apache.org/jira/browse/JCR-569?page=comments#action_12435161 ] Jukka Zitting commented on JCR-569: -----------------------------------
> Here is the design of the new Importer class. Thanks! Though I still need to ask for more clarification: Is the proposed class supposed to replace the current WorkspaceImporter, or is will it be a base class for WorkspaceImpoter? The attached patch contains just the new GenericImporter class, without any changes to WorkspaceImporter or any other class, so applying it would have no effect on Jackrabbit or the WorkspaceImporter class. Please indicate, in the patch itself, how you'd change WorkspaceImporter as that's the stated goal of this issue. As mentioned in my previous comment, I think it would be clearer if you didn't introduce a separate class, but instead refactored the internals of the existing WorkspaceImporter class. We can add the common base class as needed by the backup tool once this refactoring is done, doing it in one step just makes the proposed change more complex. A minor note: The GenericImporter javadoc says: "imports an XML document". Note that the responsibility of the Importer interface is actually *not* to import the XML document itself, just to import the content submitted to it by the XML import handlers. It's a subtle but quite important difference, that is seen for example in the fact that the Importer interface has no dependencies on any XML handling, it deals solely with NodeInfo and PropInfo objects instantiated by the XML import handlers. Thus the javadoc as written is incorrect. > WorkspaceImporter Refactoring > ----------------------------- > > Key: JCR-569 > URL: http://issues.apache.org/jira/browse/JCR-569 > Project: Jackrabbit > Issue Type: Improvement > Reporter: Nicolas Toper > Attachments: GenericImporter.patch, SysViewImporter.patch > > > Hi, > As you know, I have run into an issue with the backup tool using the > WorkspaceImporter. I ended up copy/pasting large body of code since the > current WorkspaceImporter was not flexible enough for my use (in my class > called SysViewImporter). This solution was perfectly valid for Google SoC (a > lot of time constraints) but unacceptable in the long run for any project (we > hate large body of duplicate code :p). > Also, some issues have been raised with this class (i.e. jcr:root > importation, allowsSameNameSiblings issue). > Overall I feel this class is circumvoluted and really hard to understand. > For instance, the current code contains at most 5 imbricated if and uses a > lot of different ways to pass information (stacks, objects set on null). > I tried to refactor my SysViewImporter and the WorkspaceImporter but it > implies a new code for the WorkspaceImporter and the SysViewImporter. Here is > its skeleton! I first wanted to gather the community feedback before stepping > in. I tried moving all "work code" away from the startNode method and > reorganise it for readilibility. > Please give me your feedback. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
