Mailing-List: contact dev-help@jackrabbit.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@jackrabbit.apache.org
Message-ID: <17196192.1172138705823.JavaMail.jira@brutus>
Date: Thu, 22 Feb 2007 02:05:05 -0800 (PST)
From: "Jukka Zitting (JIRA)" <jira@apache.org>
To: dev@jackrabbit.apache.org
Subject: [jira] Commented: (JCR-556) Refactoring of the BackupTool
In-Reply-To: <11190149.1157126362484.JavaMail.jira@brutus>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ 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.