jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Dürig <mdue...@apache.org>
Subject Re: svn commit: r1311085 - in /jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk: core/MicroKernelImpl.java model/AbstractCommit.java model/Commit.java model/CommitBuilder.java model/MutableCommit.java model/StoredCommit.java
Date Mon, 09 Apr 2012 11:18:01 GMT

On 8.4.12 21:31, stefan@apache.org wrote:
> Author: stefan
> Date: Sun Apr  8 20:31:50 2012
> New Revision: 1311085
>
> URL: http://svn.apache.org/viewvc?rev=1311085&view=rev
> Log:
> OAK-43: Incomplete journal when move and copy operations are involved

[...]

> @@ -222,19 +129,29 @@ public class CommitBuilder {
>               newCommit.setParentId(baseRevId);
>               newCommit.setCommitTS(System.currentTimeMillis());
>               newCommit.setMsg(msg);
> +            StringBuilder diff = new StringBuilder();
> +            for (Change change : changeLog) {
> +                if (diff.length()>  0) {
> +                    diff.append('\n');
> +                }
> +                diff.append(change.asDiff());
> +            }
> +            newCommit.setChanges(diff.toString());

Why going through all that trouble here? Why not storing the json diff 
passed to the commit method?

Michael


>               newCommit.setRootNodeId(rootNodeId);
>               newRevId = store.putHeadCommit(newCommit);
>           } finally {
>               store.unlockHead();
>           }
>
> -        // reset instance in order to be reusable
> +        // reset instance
>           staged.clear();
>           changeLog.clear();
>
>           return newRevId;
>       }
>
> +    //--------------------------------------------------------<  inner classes>
> +
>       MutableNode getOrCreateStagedNode(String nodePath) throws Exception {
>           MutableNode node = staged.get(nodePath);
>           if (node == null) {
> @@ -418,23 +335,79 @@ public class CommitBuilder {
>       }
>
>       //--------------------------------------------------------<  inner classes>
> +
> +    public static class NodeTree {
> +        public Map<String, String>  props = new HashMap<String, String>();
> +        public Map<String, NodeTree>  nodes = new HashMap<String, NodeTree>();
> +
> +        void toJson(StringBuffer buf) {
> +            toJson(buf, this);
> +        }
> +
> +        private static void toJson(StringBuffer buf, NodeTree node) {
> +            buf.append('{');
> +            for (String name : node.props.keySet()) {
> +                if (buf.charAt(buf.length() - 1) != '{')  {
> +                    buf.append(',');
> +                }
> +                buf.append('"').append(name).append("\":").append(node.props.get(name));
> +            }
> +            for (String name : node.nodes.keySet()) {
> +                if (buf.charAt(buf.length() - 1) != '{')  {
> +                    buf.append(',');
> +                }
> +                buf.append('"').append(name).append("\":");
> +                toJson(buf, node.nodes.get(name));
> +            }
> +            buf.append('}');
> +        }
> +    }
> +
>       abstract class Change {
>           abstract void apply() throws Exception;
> +        abstract String asDiff();
>       }
>
>       class AddNode extends Change {
>           String parentNodePath;
>           String nodeName;
> -        Map<String, String>  properties;
> +        NodeTree node;
>
> -        AddNode(String parentNodePath, String nodeName, Map<String, String>  properties)
{
> +        AddNode(String parentNodePath, String nodeName, NodeTree node) {
>               this.parentNodePath = parentNodePath;
>               this.nodeName = nodeName;
> -            this.properties = properties;
> +            this.node = node;
>           }
>
> +        @Override
>           void apply() throws Exception {
> -            addNode(parentNodePath, nodeName, properties);
> +            recursiveAddNode(parentNodePath, nodeName, node);
> +        }
> +
> +        @Override
> +        String asDiff() {
> +            StringBuffer diff = new StringBuffer("+");
> +            diff.append('"').append(PathUtils.concat(parentNodePath, nodeName)).append("\":");
> +            node.toJson(diff);
> +            return diff.toString();
> +        }
> +
> +        private void recursiveAddNode(String parentPath, String name, NodeTree node)
throws Exception {
> +            MutableNode modParent = getOrCreateStagedNode(parentPath);
> +            if (modParent.getChildNodeEntry(name) != null) {
> +                throw new Exception("there's already a child node with name '" + name
+ "'");
> +            }
> +            String newPath = PathUtils.concat(parentPath, name);
> +            MutableNode newChild = new MutableNode(store, newPath);
> +            newChild.getProperties().putAll(node.props);
> +
> +            // id will be computed on commit
> +            modParent.add(new ChildNode(name, null));
> +            staged.put(newPath, newChild);
> +
> +            for (String childName : node.nodes.keySet()) {
> +                recursiveAddNode(PathUtils.concat(parentPath, name), childName, node.nodes.get(childName));
> +            }
>           }
>       }
>
> @@ -445,8 +418,25 @@ public class CommitBuilder {
>               this.nodePath = nodePath;
>           }
>
> +        @Override
>           void apply() throws Exception {
> -            removeNode(nodePath);
> +            String parentPath = PathUtils.getParentPath(nodePath);
> +            String nodeName = PathUtils.getName(nodePath);
> +
> +            MutableNode parent = getOrCreateStagedNode(parentPath);
> +            if (parent.remove(nodeName) == null) {
> +                throw new NotFoundException(nodePath);
> +            }
> +
> +            // update staging area
> +            removeStagedNodes(nodePath);
> +        }
> +
> +        @Override
> +        String asDiff() {
> +            StringBuffer diff = new StringBuffer("-");
> +            diff.append('"').append(nodePath).append('"');
> +            return diff.toString();
>           }
>       }
>
> @@ -459,8 +449,48 @@ public class CommitBuilder {
>               this.destPath = destPath;
>           }
>
> +        @Override
>           void apply() throws Exception {
> -            moveNode(srcPath, destPath);
> +            if (PathUtils.isAncestor(srcPath, destPath)) {
> +                throw new Exception("target path cannot be descendant of source path:
" + destPath);
> +            }
> +
> +            String srcParentPath = PathUtils.getParentPath(srcPath);
> +            String srcNodeName = PathUtils.getName(srcPath);
> +
> +            String destParentPath = PathUtils.getParentPath(destPath);
> +            String destNodeName = PathUtils.getName(destPath);
> +
> +            MutableNode srcParent = getOrCreateStagedNode(srcParentPath);
> +            if (srcParentPath.equals(destParentPath)) {
> +                if (srcParent.getChildNodeEntry(destNodeName) != null) {
> +                    throw new Exception("node already exists at move destination path:
" + destPath);
> +                }
> +                if (srcParent.rename(srcNodeName, destNodeName) == null) {
> +                    throw new NotFoundException(srcPath);
> +                }
> +            } else {
> +                ChildNode srcCNE = srcParent.remove(srcNodeName);
> +                if (srcCNE == null) {
> +                    throw new NotFoundException(srcPath);
> +                }
> +
> +                MutableNode destParent = getOrCreateStagedNode(destParentPath);
> +                if (destParent.getChildNodeEntry(destNodeName) != null) {
> +                    throw new Exception("node already exists at move destination path:
" + destPath);
> +                }
> +                destParent.add(new ChildNode(destNodeName, srcCNE.getId()));
> +            }
> +
> +            // update staging area
> +            moveStagedNodes(srcPath, destPath);
> +        }
> +
> +        @Override
> +        String asDiff() {
> +            StringBuffer diff = new StringBuffer(">");
> +            diff.append('"').append(srcPath).append("\":\"").append(destPath).append('"');
> +            return diff.toString();
>           }
>       }
>
> @@ -473,8 +503,36 @@ public class CommitBuilder {
>               this.destPath = destPath;
>           }
>
> +        @Override
>           void apply() throws Exception {
> -            copyNode(srcPath, destPath);
> +            String srcParentPath = PathUtils.getParentPath(srcPath);
> +            String srcNodeName = PathUtils.getName(srcPath);
> +
> +            String destParentPath = PathUtils.getParentPath(destPath);
> +            String destNodeName = PathUtils.getName(destPath);
> +
> +            MutableNode srcParent = getOrCreateStagedNode(srcParentPath);
> +            ChildNode srcCNE = srcParent.getChildNodeEntry(srcNodeName);
> +            if (srcCNE == null) {
> +                throw new NotFoundException(srcPath);
> +            }
> +
> +            MutableNode destParent = getOrCreateStagedNode(destParentPath);
> +            destParent.add(new ChildNode(destNodeName, srcCNE.getId()));
> +
> +            if (srcCNE.getId() == null) {
> +                // a 'new' node is being copied
> +
> +                // update staging area
> +                copyStagedNodes(srcPath, destPath);
> +            }
> +        }
> +
> +        @Override
> +        String asDiff() {
> +            StringBuffer diff = new StringBuffer("*");
> +            diff.append('"').append(srcPath).append("\":\"").append(destPath).append('"');
> +            return diff.toString();
>           }
>       }
>
> @@ -489,22 +547,23 @@ public class CommitBuilder {
>               this.propValue = propValue;
>           }
>
> +        @Override
>           void apply() throws Exception {
> -            setProperty(nodePath, propName, propValue);
> -        }
> -    }
> +            MutableNode node = getOrCreateStagedNode(nodePath);
>
> -    class SetProperties extends Change {
> -        String nodePath;
> -        Map<String, String>  properties;
> -
> -        SetProperties(String nodePath, Map<String, String>  properties) {
> -            this.nodePath = nodePath;
> -            this.properties = properties;
> +            Map<String, String>  properties = node.getProperties();
> +            if (propValue == null) {
> +                properties.remove(propName);
> +            } else {
> +                properties.put(propName, propValue);
> +            }
>           }
>
> -        void apply() throws Exception {
> -            setProperties(nodePath, properties);
> +        @Override
> +        String asDiff() {
> +            StringBuffer diff = new StringBuffer("^");
> +            diff.append('"').append(PathUtils.concat(nodePath, propName)).append("\":").append(propValue);
> +            return diff.toString();
>           }
>       }
>   }
>
> Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/MutableCommit.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/MutableCommit.java?rev=1311085&r1=1311084&r2=1311085&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/MutableCommit.java
(original)
> +++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/MutableCommit.java
Sun Apr  8 20:31:50 2012
> @@ -39,6 +39,7 @@ public class MutableCommit extends Abstr
>           setRootNodeId(other.getRootNodeId());
>           setCommitTS(other.getCommitTS());
>           setMsg(other.getMsg());
> +        setChanges(other.getChanges());
>           this.id = other.getId();
>       }
>
> @@ -57,7 +58,11 @@ public class MutableCommit extends Abstr
>       public void setMsg(String msg) {
>           this.msg = msg;
>       }
> -
> +
> +    public void setChanges(String changes) {
> +        this.changes = changes;
> +    }
> +
>       /**
>        * Return the commit id.
>        *
>
> Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/StoredCommit.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/StoredCommit.java?rev=1311085&r1=1311084&r2=1311085&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/StoredCommit.java
(original)
> +++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/StoredCommit.java
Sun Apr  8 20:31:50 2012
> @@ -29,17 +29,19 @@ public class StoredCommit extends Abstra
>           Id rootNodeId = new Id(binding.readBytesValue("rootNodeId"));
>           long commitTS = binding.readLongValue("commitTS");
>           String msg = binding.readStringValue("msg");
> +        String changes = binding.readStringValue("changes");
>           String parentId = binding.readStringValue("parentId");
>           return new StoredCommit(id, "".equals(parentId) ? null : Id.fromString(parentId),
> -                commitTS, rootNodeId, "".equals(msg) ? null : msg);
> +                commitTS, rootNodeId, "".equals(msg) ? null : msg, changes);
>       }
>
> -    public StoredCommit(Id id, Id parentId, long commitTS, Id rootNodeId, String msg)
{
> +    public StoredCommit(Id id, Id parentId, long commitTS, Id rootNodeId, String msg,
String changes) {
>           this.id = id;
>           this.parentId = parentId;
>           this.commitTS = commitTS;
>           this.rootNodeId = rootNodeId;
>           this.msg = msg;
> +        this.changes = changes;
>       }
>
>       public StoredCommit(Id id, Commit commit) {
>
>


Mime
View raw message