[jira] Commented: (JCR-569) WorkspaceImporter Refactoring

["Nicolas Toper (JIRA)" <ji...@apache.org>]

    [ 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