jackrabbit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jukka Zitting (JIRA)" <j...@apache.org>
Subject [jira] Commented: (JCR-556) Refactoring of the BackupTool
Date Thu, 22 Feb 2007 10:05:05 GMT

    [ https://issues.apache.org/jira/browse/JCR-556?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12474961
] 

Jukka Zitting commented on JCR-556:
-----------------------------------

Re: VisitorPattern200207.patch; Looks good, details below:

The main issue is about visiting NodeReferences, basically all the information required to
recreate the NodeReferences is already included in the Node- and PropertyStates, as the NodeReferences
objects simply list the PropertyIds of all PropertyStates that have reference values pointing
to given Nodes. If a visitor needs that reference information, it can recreate the NodeReferences
objects on the fly based on the visited PropertyState instances. Thus I'd drop the visit(NodeReferences)
method from the visitor interface.

The code flow in accept(String, ItemStateVisitor) is a bit cumbersome, though it's perhaps
just me and my taste. The "if (sql.equals(...))" statements seem a bit unnecessary since the
state information already exists higher up the call chain at accept(ItemStateVisitor). I'd
rather duplicate the JDBC statement handling code than use such a switch structure.

There is a somewhat essential issue regarding the extra whitespace being added to SimpleDbPersistenceManager.
Since you make no other changes to that class, it would be better if the patch didn't touch
the file at all.

Alltogether it's good work, feel free to continue based on that. I'll however not apply the
changes to the Jackrabbit trunk until there's also some client code that uses the new interfaces.

> Refactoring of the BackupTool
> -----------------------------
>
>                 Key: JCR-556
>                 URL: https://issues.apache.org/jira/browse/JCR-556
>             Project: Jackrabbit
>          Issue Type: Improvement
>            Reporter: Nicolas Toper
>            Priority: Minor
>         Attachments: BIO.patch, patch-backup-040906.txt, patch-jr-010906-NodeVersionHistoriesUpdatableStateManager.txt,
patch-jr-010906-PropInfo.txt, patch-jr-010906-RepositoryImpl.txt, patch-jr-010906-SysViewImporter.txt,
patch-jr-010906-VersionManagerImpl.txt, PropInfo.patch, VisitorPattern200207.patch
>
>
> The BackupTool has still some refactoring to perform. I will propose the patch on this
new issue since we cannot change the JCR-442 (Google wants to see a place where all the Google
SoC work is).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message