jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Guggisberg <stefan.guggisb...@gmail.com>
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 13:35:12 GMT
On Mon, Apr 9, 2012 at 1:18 PM, Michael Dürig <mduerig@apache.org> wrote:
>
> 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?

i thought about this but decided against it because the paths within
the passed diff might need to be normalized.
furthermore i would have to pass the raw diff to the CommitBuilder
instance which feels wrong.

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