You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by md...@apache.org on 2013/04/24 14:44:48 UTC

svn commit: r1471389 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak: plugins/memory/MemoryNodeBuilder.java plugins/memory/MutableNodeState.java spi/state/NodeBuilder.java

Author: mduerig
Date: Wed Apr 24 12:44:48 2013
New Revision: 1471389

URL: http://svn.apache.org/r1471389
Log:
OAK-781: Clarify / fix effects of MISSING_NODE as base state of NodeBuilder
Javadoc

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MutableNodeState.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java?rev=1471389&r1=1471388&r2=1471389&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java Wed Apr 24 12:44:48 2013
@@ -34,33 +34,20 @@ import org.apache.jackrabbit.oak.spi.sta
 /**
  * In-memory node state builder.
  * <p>
- * TODO: The following description is somewhat out of date
+ * A {@code MemoryNodeBuilder} instance tracks uncommitted changes without
+ * relying on weak references or requiring hard references on the entire
+ * accessed subtree. It does this by relying on {@code MutableNodeState}
+ * instances for tracking <em>uncommitted changes</em>. A child builders
+ * keeps a reference to its parent builder and knows it's name. Before
+ * each access the builder checks the mutable state of its parent for
+ * relevant changes and updates its own mutable state.
  * <p>
- * The following two builder states are used
- * to properly track uncommitted chances without relying on weak references
- * or requiring hard references on the entire accessed subtree:
- * <dl>
- *   <dt>unmodified</dt>
- *   <dd>
- *     A child builder with no content changes starts in this state.
- *     It keeps a reference to the parent builder and knows it's name for
- *     use when connecting. Before each access the unconnected builder
- *     checks the parent for relevant changes to connect to. As long as
- *     there are no such changes, the builder remains unconnected and
- *     uses the immutable base state to respond to any content accesses.
- *   </dd>
- *   <dt>connected</dt>
- *   <dd>
- *     Once a child node is first modified, it switches it's internal
- *     state from the immutable base state to a mutable one and records
- *     a hard reference to that state in the mutable parent state. After
- *     that the parent reference is cleared and no more state checks are
- *     made. Any other builder instances that refer to the same child node
- *     will update their internal states to point to that same mutable
- *     state instance and thus become connected at next access.
- *     A root state builder is always connected.
- *   </dd>
- * </dl>
+ * A {@code MutableNodeState} instance does not keep a reference to its
+ * parent state. It only keeps references to children that have been
+ * modified. Instances representing an unmodified child are created on
+ * the fly without keeping a reference. This effectively ensures that
+ * such an instance can be GC'ed once no node builder references it
+ * anymore.
  */
 public class MemoryNodeBuilder implements NodeBuilder {
 
@@ -80,26 +67,40 @@ public class MemoryNodeBuilder implement
      */
     private final MemoryNodeBuilder root;
 
+    /**
+     * Internal revision counter for the base state of this builder. The counter
+     * is incremented in the root builder whenever its base state is reset.
+     * Each builder instance has its own copy of this revision counter for
+     * quickly checking whether its base state needs updating.
+     * @see #reset(org.apache.jackrabbit.oak.spi.state.NodeState)
+     */
     private long baseRevision;
 
     /**
      * The base state of this builder, possibly non-existent if this builder
      * represents a new node that didn't yet exist in the base content tree.
+     * @see #base()
      */
     private NodeState base;
 
+    /**
+     * Internal revision counter for the head state of this builder. The counter
+     * is incremented in the root builder whenever anything changes in the tree
+     * below. Each builder instance has its own copy of this revision counter for
+     * quickly checking whether its head state needs updating.
+     */
     private long headRevision;
 
     /**
-     * The shared mutable state of connected builder instances, or
-     * {@code null} until this builder has been connected.
+     * The shared mutable state this builder.
+     * @see #write()
+     * @see #read()
      */
     private MutableNodeState head;
 
     /**
-     * Creates a new in-memory node state builder.
-     *
-     * @param parent parent node state builder
+     * Creates a new in-memory child builder.
+     * @param parent parent builder
      * @param name name of this node
      */
     private MemoryNodeBuilder(MemoryNodeBuilder parent, String name) {
@@ -109,8 +110,8 @@ public class MemoryNodeBuilder implement
     }
 
     /**
-     * Creates a new in-memory node state builder.
-     *
+     * Creates a new in-memory node state builder rooted at
+     * and based on the passed {@code base} state.
      * @param base base state of the new builder
      */
     public MemoryNodeBuilder(@Nonnull NodeState base) {
@@ -118,9 +119,11 @@ public class MemoryNodeBuilder implement
         this.name = null;
         this.root = this;
 
+        // ensure base is updated on next call to base()
         this.baseRevision = 1;
         this.base = checkNotNull(base);
 
+        // ensure head is updated on next call to read() or write()
         this.headRevision = 1;
         this.head = new MutableNodeState(this.base);
     }
@@ -135,6 +138,11 @@ public class MemoryNodeBuilder implement
         }
     }
 
+    /**
+     * Update the base state of this builder by recursively retrieving it
+     * from the parent builder.
+     * @return  base state of this builder
+     */
     @Nonnull
     private NodeState base() {
         if (root.baseRevision != baseRevision) {
@@ -144,6 +152,11 @@ public class MemoryNodeBuilder implement
         return base;
     }
 
+    /**
+     * Update the head state of this builder by recursively retrieving it
+     * from the parent builder.
+     * @return  head state of this builder
+     */
     @Nonnull
     private MutableNodeState read() {
         if (headRevision != root.headRevision) {
@@ -154,11 +167,21 @@ public class MemoryNodeBuilder implement
         return head;
     }
 
+    /**
+     * Update the head state of this builder by recursively retrieving it
+     * from the parent builder and increment the head revision of the root
+     * builder ensuring subsequent calls to {@link #read()} result in updating
+     * of the respective head states.
+     * @return  head state of this builder
+     */
     @Nonnull
     private MutableNodeState write() {
         return write(root.headRevision + 1);
     }
 
+    /**
+     * Recursive helper method to {@link #write()}. Don't call directly.
+     */
     @Nonnull
     private MutableNodeState write(long newRevision) {
         if (headRevision != newRevision && !isRoot()) {

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MutableNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MutableNodeState.java?rev=1471389&r1=1471388&r2=1471389&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MutableNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MutableNodeState.java Wed Apr 24 12:44:48 2013
@@ -41,10 +41,12 @@ import org.apache.jackrabbit.oak.spi.sta
 import org.apache.jackrabbit.oak.spi.state.NodeStateDiff;
 
 /**
- * The <em>mutable</em> state being built. Instances of this class
- * are never passed beyond the containing {@code MemoryNodeBuilder},
- * so it's not a problem that we intentionally break the immutability
- * assumption of the {@link org.apache.jackrabbit.oak.spi.state.NodeState} interface.
+ * A <em>mutable</em> state being built.
+ *
+ * Instances of this class are never passed beyond the containing
+ * {@link MemoryNodeBuilder}, so it's not a problem that we intentionally
+ * break the immutability assumption of the
+ * {@link org.apache.jackrabbit.oak.spi.state.NodeState} interface.
  */
 class MutableNodeState extends AbstractNodeState {
 
@@ -213,6 +215,11 @@ class MutableNodeState extends AbstractN
                 !nodes.containsKey(name) && base.getChildNode(name).exists();
     }
 
+    /**
+     * Get and optionally connect a potentially non existing child
+     * node of a given {@code name}. Connected child nodes are kept
+     * in the list of modified child nodes of this node.
+     */
     MutableNodeState getChildNode(String name, boolean connect) {
         assert base != null;
 
@@ -222,7 +229,7 @@ class MutableNodeState extends AbstractN
         }
 
         if (nodes.containsKey(name)) {
-            // deleted
+            // deleted: shadow if connect, otherwise non existing
             child = new MutableNodeState(connect);
         } else {
             child = new MutableNodeState(base.getChildNode(name));
@@ -234,6 +241,14 @@ class MutableNodeState extends AbstractN
         return child;
     }
 
+    /**
+     * Equivalent to
+     * <pre>
+     *   MutableNodeState child = getChildNode(name, true);
+     *   child.reset(state);
+     *   return child;
+     * </pre>
+     */
     @Nonnull
     MutableNodeState setChildNode(String name, NodeState state) {
         // FIXME better implementation, which doesn't set the base state twice
@@ -242,6 +257,18 @@ class MutableNodeState extends AbstractN
         return child;
     }
 
+    /**
+     * Determine whether this node state is modified wrt. the passed
+     * {@code before} state.
+     * <p>
+     * A node state is modified if it either has not the same properties
+     * or has not the same child nodes as a {@code before} state. A node
+     * state has the same properties as a {@code before} state iff its
+     * set of properties is equal to the set of properties of
+     * {@code before}. A node state has the same child nodes as a
+     * {@code before} state iff its set of child node names is equal to
+     * the set of child node names of {@code before}.
+     */
     boolean isModified(NodeState before) {
         if (nodes.isEmpty() && properties.isEmpty()) {
             return false;
@@ -268,6 +295,11 @@ class MutableNodeState extends AbstractN
 
     }
 
+    /**
+     * Remove the child node with the given {@code name}.
+     * @param name  name of the child node to remove
+     * @return  {@code true} if a child node {@code name} existed, {@code false} otherwise.
+     */
     boolean removeChildNode(String name) {
         assert base != null;
 
@@ -279,6 +311,11 @@ class MutableNodeState extends AbstractN
         }
     }
 
+    /**
+     * Remove the property of the given {@code name}.
+     * @param name  name of the property to remove
+     * @return  {@code true} if a property {@code name} existed, {@code false} otherwise.
+     */
     boolean removeProperty(String name) {
         assert base != null;
 
@@ -290,6 +327,9 @@ class MutableNodeState extends AbstractN
         }
     }
 
+    /**
+     * Set the value of a property
+     */
     void setProperty(PropertyState property) {
         properties.put(property.getName(), property);
     }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java?rev=1471389&r1=1471388&r2=1471389&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java Wed Apr 24 12:44:48 2013
@@ -24,6 +24,48 @@ import org.apache.jackrabbit.oak.api.Typ
 
 /**
  * Builder interface for constructing new {@link NodeState node states}.
+ * <p>
+ * A node builder can be thought of as a mutable version of a node state.
+ * In addition to property and child node access methods like the ones that
+ * are already present in the NodeState interface, the {@code NodeBuilder}
+ * interface contains the following key methods:
+ * <ul>
+ * <li>The {@code setProperty} and {@code removeProperty} methods for
+ *     modifying properties</li>
+ * <li>The {@code getChildNode} method for accessing or modifying an existing
+ *     subtree</li>
+ * <li>The {@code setChildNode} and {@code removeChildNode} methods for adding,
+ *     replacing or removing a subtree</li>
+ * <li>The {@code exists} method for checking whether the node represented by
+ *     a builder exists or is accessible</li>
+ * <li>The {@code getNodeState} method for getting a frozen snapshot of the
+ *     modified content tree</li>
+ * </ul>
+ * All the builders acquired from the same root builder instance are linked so
+ * that changes made through one instance automatically become visible in the other
+ * builders. For example:
+ * <pre>
+ *     NodeBuilder rootBuilder = root.builder();
+ *     NodeBuilder fooBuilder = rootBuilder.getChildNode("foo");
+ *     NodeBuilder barBuilder = fooBuilder.getChildNode("bar");
+ *
+ *     assert !barBuilder.getBoolean("x");
+ *     fooBuilder.getChildNode("bar").setProperty("x", Boolean.TRUE);
+ *     assert barBuilder.getBoolean("x");
+ *
+ *     assert barBuilder.exists();
+ *     fooBuilder.removeChildNode("bar");
+ *     assert !barBuilder.exists();
+ * </pre>
+ * The {@code getNodeState} method returns a frozen, immutable snapshot of the current
+ * state of the builder. Providing such a snapshot can be somewhat expensive especially
+ * if there are many changes in the builder, so the method should generally only be used
+ * as the last step after all intended changes have been made. Meanwhile the accessors
+ * in the {@code NodeBuilder} interface can be used to provide efficient read access to
+ * the current state of the tree being modified.
+ * <p>
+ * The node states constructed by a builder often retain an internal reference to the base
+ * state used by the builder. This allows common node state comparisons to perform really.
  */
 public interface NodeBuilder {