You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by "Nicolas Toper (JIRA)" <ji...@apache.org> on 2006/09/01 17:59:22 UTC

[jira] Created: (JCR-556) Refactoring of the BackupTool

Refactoring of the BackupTool
-----------------------------

                 Key: JCR-556
                 URL: http://issues.apache.org/jira/browse/JCR-556
             Project: Jackrabbit
          Issue Type: Improvement
            Reporter: Nicolas Toper
            Priority: Minor


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

        

[jira] Commented: (JCR-556) Refactoring of the BackupTool

Posted by "Nicolas Toper (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/JCR-556?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12475018 ] 

Nicolas Toper commented on JCR-556:
-----------------------------------

Stefan,

You are right. It is more elegant. I'll update the code and rewrite Javadoc parts.

Thanks
Nico

> 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, VisitorPattern22022007-2.patch, VisitorPattern22022007.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.


[jira] Updated: (JCR-556) Refactoring of the BackupTool

Posted by "Nicolas Toper (JIRA)" <ji...@apache.org>.
     [ http://issues.apache.org/jira/browse/JCR-556?page=all ]

Nicolas Toper updated JCR-556:
------------------------------

    Attachment: BIO.patch

Add a flag autoCreate to know whether or not to create autocreate Node. As suggested by Jukka, it avoids to create a specific class to restore the NodeVersionHistories.

> Refactoring of the BackupTool
> -----------------------------
>
>                 Key: JCR-556
>                 URL: http://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
>
>
> 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.
-
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

        

[jira] Commented: (JCR-556) Refactoring of the BackupTool

Posted by "Nicolas Toper (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/JCR-556?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12474995 ] 

Nicolas Toper commented on JCR-556:
-----------------------------------

Hi,

Please disregard the last patch. There is still some tabs and trailing spaces. I will post the good one this afternoon.

Nico

> 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, VisitorPattern22022007.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.


[jira] Commented: (JCR-556) Refactoring of the BackupTool

Posted by "Stefan Guggisberg (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/JCR-556?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12475037 ] 

Stefan Guggisberg commented on JCR-556:
---------------------------------------

nicolas,
there are still some issues. streams and result sets are not properly closed.
there are also some minor issues like inaccurate exception msg & inline 
comments that don't seem to make sense..

here's my slightly revised suggestion:

    /**
     * {@inheritDoc}
     */
    public void accept(ItemStateVisitor visitor) throws ItemStateException {
        ResultSet rs = null;
        InputStream in = null;
        try {
            // visit node states
            Statement stmt = executeStmt(nodeStateSelectAllSQL, new Object[0]);
            rs = stmt.getResultSet();
            while (rs.next()) {
                String id = rs.getString(1);
                NodeState state = createNew(NodeId.valueOf(id));
                in = rs.getBinaryStream(2);
                Serializer.deserialize(state, in);
                visitor.visit(state);
                closeStream(in);
                in = null;
            }
            closeResultSet(rs);
            rs = null;

            // visit property states
            stmt = executeStmt(propertyStateSelectAllSQL, new Object[0]);
            rs = stmt.getResultSet();
            while (rs.next()) {
                String id = rs.getString(1);
                PropertyState state = createNew(PropertyId.valueOf(id));
                in = rs.getBinaryStream(2);
                Serializer.deserialize(state, in, blobStore);
                visitor.visit(state);
                closeStream(in);
                in = null;
            }
        } catch (Exception e) {
            String msg = "failed to visit item states";
            log.error(msg, e);
            throw new ItemStateException(msg, e);
        } finally {
            closeStream(in);
            closeResultSet(rs);
        }
    }



> 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, VisitorPattern22022007-2.patch, VisitorPattern22022007-3.patch, VisitorPattern22022007.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.


[jira] Updated: (JCR-556) Refactoring of the BackupTool

Posted by "Nicolas Toper (JIRA)" <ji...@apache.org>.
     [ http://issues.apache.org/jira/browse/JCR-556?page=all ]

Nicolas Toper updated JCR-556:
------------------------------

    Attachment: patch-jr-010906-SysViewImporter.txt

SysViewImporter: takes a system view XML document and import it as it is in an empty repository. It is heavily based on the WorkspaceImporter but doesn't extend it (I would have to put too many methods in protected for this; tell me if you would be ok with this change though). Here are the differences:
      1/ it escapes the isProtected check and doesn't autoCreate nodes. It checks if there are some contents in the destination, if yes it throws an exception.
      2/ You have more control through the constructor than the WorkspaceImporter since you specify which BatchedItemOperations you want to use.
      3/ There are no conflicts/replace so basically, it just creates nodes and so is a lot simpler thant the WorkspaceImporter.

> Refactoring of the BackupTool
> -----------------------------
>
>                 Key: JCR-556
>                 URL: http://issues.apache.org/jira/browse/JCR-556
>             Project: Jackrabbit
>          Issue Type: Improvement
>            Reporter: Nicolas Toper
>            Priority: Minor
>         Attachments: 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
>
>
> 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.
-
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

        

[jira] Updated: (JCR-556) Refactoring of the BackupTool

Posted by "Nicolas Toper (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/JCR-556?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Nicolas Toper updated JCR-556:
------------------------------

    Attachment: VisitorPattern22022007-2.patch

VisitorPattern22022007-2.patch is checkstyle compliant. 

> 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, VisitorPattern22022007-2.patch, VisitorPattern22022007.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.


[jira] Updated: (JCR-556) Refactoring of the BackupTool

Posted by "Nicolas Toper (JIRA)" <ji...@apache.org>.
     [ http://issues.apache.org/jira/browse/JCR-556?page=all ]

Nicolas Toper updated JCR-556:
------------------------------

    Attachment: patch-jr-010906-VersionManagerImpl.txt

This patch adds importVersions in the VersionManagerImpl.

> Refactoring of the BackupTool
> -----------------------------
>
>                 Key: JCR-556
>                 URL: http://issues.apache.org/jira/browse/JCR-556
>             Project: Jackrabbit
>          Issue Type: Improvement
>            Reporter: Nicolas Toper
>            Priority: Minor
>         Attachments: 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
>
>
> 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.
-
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

        

[jira] Commented: (JCR-556) Refactoring of the BackupTool

Posted by "Stefan Guggisberg (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/JCR-556?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12475012 ] 

Stefan Guggisberg commented on JCR-556:
---------------------------------------

> Nicolas Toper updated JCR-556:
> ------------------------------
> 
> I didn't change the code flow. IMO both ways has drawbacks. 

what is the drawback of jukka's suggestion? the code would imo be cleaner.

btw:  there's no need to synchronize on the sql stmt  since there are no bound paramaters.

here's my suggestion:

    //-----------------------------------------< VisitableItemStateCollection >
    /**
     * {@inheritDoc}
     */
    public void accept(ItemStateVisitor visitor) throws ItemStateException {
        ResultSet rs = null;
        InputStream in = null;
        try {
            // visit node states
            Statement stmt = executeStmt(nodeStateSelectAllSQL, new Object[0]);
            rs = stmt.getResultSet();
            while (rs.next()) {
                String id = rs.getString(1);
                NodeState state = createNew(NodeId.valueOf(id));
                in = rs.getBinaryStream(2);
                Serializer.deserialize(state, in);
                visitor.visit(state);
            }
            closeStream(in);
            closeResultSet(rs);

            // visit property states
            stmt = executeStmt(propertyStateSelectAllSQL, new Object[0]);
            rs = stmt.getResultSet();
            while (rs.next()) {
                String id = rs.getString(1);
                PropertyState state = createNew(PropertyId.valueOf(id));
                in = rs.getBinaryStream(2);
                Serializer.deserialize(state, in, blobStore);
                visitor.visit(state);
            }
        } catch (Exception e) {
            String msg = "failed to visit item states";
            log.error(msg, e);
            throw new ItemStateException(msg, e);
        } finally {
            closeStream(in);
            closeResultSet(rs);
        }
    }

the above code assumes the following sql stmts:

+        nodeStateSelectAllSQL = "select NODE_ID, NODE_DATA from " + schemaObjectPrefix + "NODE";
+        propertyStateSelectAllSQL = "select PROP_ID, PROP_DATA from " + schemaObjectPrefix + "PROP";


there are also some minor javaodc issues: 
e.g. javadoc of  generic VisitableItemStateCollection & ItemStateVisitor refer to 'persistence manager'

> 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, VisitorPattern22022007-2.patch, VisitorPattern22022007.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.


[jira] Updated: (JCR-556) Refactoring of the BackupTool

Posted by "Nicolas Toper (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/JCR-556?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Nicolas Toper updated JCR-556:
------------------------------

    Attachment: VisitorPattern22022007-3.patch

VisitorPattern22022007-3.patch implements all the required updates. My visitor class visits all the states with it.

Thank you very much for your remark. Unless you see anything else, I will now finish the backup and restore code which use the Visitor.

Nico

> 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, VisitorPattern22022007-2.patch, VisitorPattern22022007-3.patch, VisitorPattern22022007.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.


[jira] Updated: (JCR-556) Refactoring of the BackupTool

Posted by "Nicolas Toper (JIRA)" <ji...@apache.org>.
     [ http://issues.apache.org/jira/browse/JCR-556?page=all ]

Nicolas Toper updated JCR-556:
------------------------------

    Attachment: patch-backup-040906.txt

Hi,

Here are some refactoring to the backup tool. This goes in the contrib package for the backup tool. It is mainly clean up of the code (remove some unused variables and comments)

There is one issue remaining with CRC check in the zip. Maybe you can help me: I calculate a CRC for each entry. With the debugger, I see it there, but when I open the file in read mode, it is not here anymore. Does anyone know why? It is important I believe.

Next steps are for me to code two patches:

1- To integrate Jukka's ideas

2- To implement those changes in this code (only a few lines will be changed)

BR
Nico

> Refactoring of the BackupTool
> -----------------------------
>
>                 Key: JCR-556
>                 URL: http://issues.apache.org/jira/browse/JCR-556
>             Project: Jackrabbit
>          Issue Type: Improvement
>            Reporter: Nicolas Toper
>            Priority: Minor
>         Attachments: 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
>
>
> 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.
-
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

        

[jira] Commented: (JCR-556) Refactoring of the BackupTool

Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
    [ 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.


[jira] Commented: (JCR-556) Refactoring of the BackupTool

Posted by "Stefan Guggisberg (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/JCR-556?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12474979 ] 

Stefan Guggisberg commented on JCR-556:
---------------------------------------

i agree with jukka's comments, i've got one more:

+    private Statement executeStmt(String sql) throws SQLException {

this is an unneccessary code duplication  of the existing method 
     
      protected Statement executeStmt(String sql, Object[] params) throws SQLException {


just pass an empty parameter array to the existing method, e.g. 

      executeStmt("select * from foo", new Object[0]);


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


[jira] Commented: (JCR-556) Refactoring of the BackupTool

Posted by "Nicolas Toper (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/JCR-556?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12475049 ] 

Nicolas Toper commented on JCR-556:
-----------------------------------

Yes my mistake.

Why don't you want to check for initialization of the persistence manager?

Nico

> 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, VisitorPattern22022007-2.patch, VisitorPattern22022007-3.patch, VisitorPattern22022007.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.


[jira] Updated: (JCR-556) Refactoring of the BackupTool

Posted by "Nicolas Toper (JIRA)" <ji...@apache.org>.
     [ http://issues.apache.org/jira/browse/JCR-556?page=all ]

Nicolas Toper updated JCR-556:
------------------------------

    Attachment: patch-jr-010906-RepositoryImpl.txt

This is a small update to RepositoryImpl. I wanted to avoid updating the interface VersionManager to add the method importVersions(...). 

This slight change allows me to since we can fetch VersionManagerImpl directly from the Repository (and we know it is always a VersionManagerImpl)

> Refactoring of the BackupTool
> -----------------------------
>
>                 Key: JCR-556
>                 URL: http://issues.apache.org/jira/browse/JCR-556
>             Project: Jackrabbit
>          Issue Type: Improvement
>            Reporter: Nicolas Toper
>            Priority: Minor
>         Attachments: 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
>
>
> 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.
-
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

        

[jira] Updated: (JCR-556) Refactoring of the BackupTool

Posted by "Nicolas Toper (JIRA)" <ji...@apache.org>.
     [ http://issues.apache.org/jira/browse/JCR-556?page=all ]

Nicolas Toper updated JCR-556:
------------------------------

    Attachment: patch-jr-010906-PropInfo.txt

This patch contains a few updates I had to PropInfo. I added three getters and switched to other getXXX to public.

> Refactoring of the BackupTool
> -----------------------------
>
>                 Key: JCR-556
>                 URL: http://issues.apache.org/jira/browse/JCR-556
>             Project: Jackrabbit
>          Issue Type: Improvement
>            Reporter: Nicolas Toper
>            Priority: Minor
>         Attachments: patch-jr-010906-NodeVersionHistoriesUpdatableStateManager.txt, patch-jr-010906-PropInfo.txt
>
>
> 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.
-
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

        

[jira] Updated: (JCR-556) Refactoring of the BackupTool

Posted by "Nicolas Toper (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/JCR-556?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Nicolas Toper updated JCR-556:
------------------------------

    Attachment: VisitorPattern22022007.patch

Hello,

Thanks for your comment.

VisitorPattern22022007.patch implements nearly all of your remarks.

I didn't change the code flow. IMO both ways has drawbacks. I feel the code is still clear because it is really simple. Since it is easy to fix later, I propose to do it later if we feel the need.

Nico 

> 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, VisitorPattern22022007.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.


[jira] Updated: (JCR-556) Refactoring of the BackupTool

Posted by "Nicolas Toper (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/JCR-556?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Nicolas Toper updated JCR-556:
------------------------------

    Attachment: Visitor2202007-final.patch

Hi,

Here is the patch with Stefan's suggestion and tested it. I have also updated my SQL requests so they could work with Stefan's code.

Thanks a lot for your help Stefan and Jukka!

Nico

> 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, Visitor2202007-final.patch, VisitorPattern200207.patch, VisitorPattern22022007-2.patch, VisitorPattern22022007-3.patch, VisitorPattern22022007.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.


[jira] Updated: (JCR-556) Refactoring of the BackupTool

Posted by "Nicolas Toper (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/JCR-556?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Nicolas Toper updated JCR-556:
------------------------------

    Attachment: VisitorPattern200207.patch

Hi,

I still haven't quit working on the backup tool. Thanks to Jukka, I have found a much easier solution to backup the workspace content: work at the persistence manager level directly. 

But to achieve this idea, I had to write a Visitor pattern to the PersistenceManager system. I needed a fast iteration on all nodes in order to backup them with a small memory footprint (restore can take longer so it is less of  an issue).  I couldn't reuse the Visitor facilities provided by the JCR API since I need to work at a much lower level.

This is the patch I now propose to the Jackrabbit community. It is composed of 3 files:

- ItemStateVisitor is an interface. A visitor of an ItemState must implement it

- VisitableItemStateCollection is an interface. Every class implementing this interface can be visited by an ItemStateVisitor. (Those classes are more general than  

- A patch to DatabasePersistenceManager implementing VisitableItemStateCollection. With it most repository are backup ready. It will be used also as a "proof of concept" code for other contributors who might want to implement the visitor interface to others peristence manager. 

If this patch is accepted, I will finish working on the backup/restore method of the backup tool (those are the last steps) and add a fallback code so all repository can be backuped albeit in a slower way.

BR,
Nico

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


[jira] Resolved: (JCR-556) Refactoring of the BackupTool

Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/JCR-556?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jukka Zitting resolved JCR-556.
-------------------------------

    Resolution: Won't Fix
      Assignee: Jukka Zitting

Resolving as Won't Fix in favour of the recent work on JCR-442.

> Refactoring of the BackupTool
> -----------------------------
>
>                 Key: JCR-556
>                 URL: https://issues.apache.org/jira/browse/JCR-556
>             Project: Jackrabbit Content Repository
>          Issue Type: Improvement
>            Reporter: Nicolas Toper
>            Assignee: Jukka Zitting
>            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, Visitor2202007-final.patch, VisitorPattern200207.patch, VisitorPattern22022007-2.patch, VisitorPattern22022007-3.patch, VisitorPattern22022007.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.


[jira] Updated: (JCR-556) Refactoring of the BackupTool

Posted by "Nicolas Toper (JIRA)" <ji...@apache.org>.
     [ http://issues.apache.org/jira/browse/JCR-556?page=all ]

Nicolas Toper updated JCR-556:
------------------------------

    Attachment: PropInfo.patch

Set a flag in PropInfo in the apply method to escape or not the protected properties. 

This flag is by default set to the usual behavior (as with BatchedItemOperations), you need to explicitaly call setSkipProtected to change it.

It avoids to createe a new class while restoring.

> Refactoring of the BackupTool
> -----------------------------
>
>                 Key: JCR-556
>                 URL: http://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
>
>
> 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.
-
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

        

[jira] Updated: (JCR-556) Refactoring of the BackupTool

Posted by "Nicolas Toper (JIRA)" <ji...@apache.org>.
     [ http://issues.apache.org/jira/browse/JCR-556?page=all ]

Nicolas Toper updated JCR-556:
------------------------------

    Attachment: patch-jr-010906-NodeVersionHistoriesUpdatableStateManager.txt

This is a new class I propose to put in o.a.j.state. 

It is designed to update easily the Node Version Histories. It is used by VersionManagerImpl.importVersions(...). The regular LocalItemStateManager was performing some checks on the protected nodes.

Depending on its other uses, we might consider refactoring it with LocalItemStateManager since they share a lot.

  

> Refactoring of the BackupTool
> -----------------------------
>
>                 Key: JCR-556
>                 URL: http://issues.apache.org/jira/browse/JCR-556
>             Project: Jackrabbit
>          Issue Type: Improvement
>            Reporter: Nicolas Toper
>            Priority: Minor
>         Attachments: patch-jr-010906-NodeVersionHistoriesUpdatableStateManager.txt
>
>
> 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.
-
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