jackrabbit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Nicolas Toper (JIRA)" <j...@apache.org>
Subject [jira] Commented: (JCR-569) WorkspaceImporter Refactoring
Date Wed, 13 Sep 2006 17:41:25 GMT
    [ http://issues.apache.org/jira/browse/JCR-569?page=comments#action_12434497 ] 
            
Nicolas Toper commented on JCR-569:
-----------------------------------

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
>         Attachments: 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

        

Mime
View raw message