[ http://issues.apache.org/jira/browse/JCR-569?page=comments#action_12442223 ] Nicolas Toper commented on JCR-569: -----------------------------------
[[ Old comment, sent by email on Wed, 13 Sep 2006 18:00:23 +0200 ]] Hi Stefan, About your first point, you are right: this is why I sent yesterday a more in depth analysis. This JIRA issue will be used as Jukka suggested it, for refactoring issue of this class only. I would like to create two classes: one generic Importer (name to be defined) and one dedicated to import a workspace. I would use the generic Importer for the backup and others could use it when they need to batch load some data (of course, it is more complex to use and set up). The WorkspaceImporter would be used for the same use case as now. The WorkspaceImporter constructor would stay the same to avoid backward compatibility issue. Here is a quick descriptive of each methods of the generic Importer. They are private since no other class need them (this class is used with the ContentHandler). private boolean checkNode(NodeInfo nodeInfo) Check if the node can be imported to the repository. For instance, it can try to delete a protected node, the same node might already exist (it can depend on the ImportUUIDBehavior used) or the node can be incorrect according to the node type in the repository. Some checks are escaped if raw mode is chosen. If the node is not correct, it puts the skip mode on true to exclude this subtree. When the subtree is over, the skip mode is put to off. If the error is serious (constraint issue for instance), an exception will be thrown. private NodeState createNode(NodeState parent, NodeInfo nodeInfo) Create the NodeState depending according to ImportUUIDBehavior flag private void createProperties(NodeState myNode, List propInfos) Create the properties depending according to ImportUUIDBehavior flag private boolean isSkipped(NodeInfo nodeInfo) if we are in skip mode return true; else false. private void postProcess(NodeState node) Initialize if raw == false, this method is not called. It will be empty for the generic Importer (we don't need to initialize any thing) but WorkspaceImporter will overload it. protected NodeState resolveUUIDConflict Resolve UUID conflict :p private boolean isAddabble(NodeState parent, NodeInfo nodeInfo) throws ConstraintViolationException, AccessDeniedException, VersionException, LockException, ItemNotFoundException, ItemExistsException, RepositoryException This method is not needed. private boolean isCorrect(NodeState parent, NodeInfo nodeInfo, List propInfos) Not needed. If you are OK with this approach, I will work on this and propose you soon a patch implementing this refactoring. BR, Nico my blog! http://www.deviant-abstraction.net !! > WorkspaceImporter Refactoring > ----------------------------- > > Key: JCR-569 > URL: http://issues.apache.org/jira/browse/JCR-569 > Project: Jackrabbit > Issue Type: Improvement > Reporter: Nicolas Toper > Assigned To: Jukka Zitting > Attachments: GenericImporter.patch, SysViewImporter.patch, > WorkspaceImporter.patch, WorkspaceImporter.patch, WorkspaceImporter.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
