Author: rhauch
Date: 2009-08-27 10:04:45 -0400 (Thu, 27 Aug 2009)
New Revision: 1179
Modified:
trunk/dna-graph/src/main/java/org/jboss/dna/graph/session/GraphSession.java
trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/AbstractJcrNode.java
trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/SessionCache.java
trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java
Log:
DNA-478 Refresh does not properly reflect changes made in other sessions
Several TCK unit tests were reporting this issue as an error. The problem stemmed from an
improperly-coded method that was attempting to refresh the children of a node, where one
(or more) of the children had been changed/created locally in the session. The method was
in fact (partially) losing those local changes and was actually corrupting the cached
state. This method has been fixed and now correctly updates those children. The TCK unit
tests were uncommented, since they now pass.
Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/session/GraphSession.java
===================================================================
--- trunk/dna-graph/src/main/java/org/jboss/dna/graph/session/GraphSession.java 2009-08-26
21:16:25 UTC (rev 1178)
+++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/session/GraphSession.java 2009-08-27
14:04:45 UTC (rev 1179)
@@ -1515,29 +1515,53 @@
assert childrenByName != null;
org.jboss.dna.graph.Node persistentNode =
persistentInfoForRefreshedNodes.getNode(location);
assert !persistentNode.getChildren().isEmpty();
- // Find the map of old Node objects keyed by their identifier ...
- Map<NodeId, Node<Payload, PropertyPayload>> oldChildren = new
HashMap<NodeId, Node<Payload, PropertyPayload>>();
- for (Node<Payload, PropertyPayload> oldChild :
childrenByName.values()) {
- oldChildren.put(oldChild.getNodeId(), oldChild);
+
+ // We need to keep the children that have been modified (or are ancestors
of modified children),
+ // so build a list of the children that SHOULD NOT be replaced with the
persistent info ...
+ Map<Location, Node<Payload, PropertyPayload>> childrenToKeep
= new HashMap<Location, Node<Payload, PropertyPayload>>();
+ for (Node<Payload, PropertyPayload> existing :
childrenByName.values()) {
+ if (existing.isChanged(true)) {
+ childrenToKeep.put(existing.getLocation(), existing);
+ } else {
+ // Otherwise, remove the child from the cache since we won't
be needing it anymore ...
+ cache.nodes.remove(existing.getNodeId());
+ assert
!cache.changeDependencies.containsKey(existing.getNodeId());
+ existing.parent = null;
+ }
}
+
+ // Now, clear the children ...
childrenByName.clear();
+
+ // And add the persistent children ...
for (Location location : persistentNode.getChildren()) {
Name childName = location.getPath().getLastSegment().getName();
- NodeId nodeId = cache.idFactory.create();
- Node<Payload, PropertyPayload> child =
oldChildren.remove(nodeId);
- if (child == null) {
- child = cache.createNode(this, nodeId, location);
- cache.nodes.put(child.getNodeId(), child);
- assert child.getName().equals(childName);
+ List<Node<Payload, PropertyPayload>> currentChildren =
childrenByName.get(childName);
+ // Find if there was an existing child that is supposed to stay ...
+ Node<Payload, PropertyPayload> existingChild =
childrenToKeep.get(location);
+ if (existingChild != null) {
+ // The existing child is supposed to stay, since it has changes
...
+ currentChildren.add(existingChild);
+ if (currentChildren.size() !=
existingChild.getPath().getLastSegment().getIndex()) {
+ // Make sure the SNS index is correct ...
+ Path.Segment segment =
cache.pathFactory.createSegment(childName, currentChildren.size());
+ existingChild.updateLocation(segment);
+ // TODO: Can the location be different? If so, doesn't
that mean that the change requests
+ // have to be updated???
+ }
+ } else {
+ // The existing child (if there was one) is to be refreshed ...
+ NodeId nodeId = cache.idFactory.create();
+ Node<Payload, PropertyPayload> replacementChild =
cache.createNode(this, nodeId, location);
+ cache.nodes.put(replacementChild.getNodeId(), replacementChild);
+ assert replacementChild.getName().equals(childName);
+ assert replacementChild.parent == this;
+ // Add it to the parent node ...
+ currentChildren.add(replacementChild);
+ // Create a segment with the SNS ...
+ Path.Segment segment = cache.pathFactory.createSegment(childName,
currentChildren.size());
+ replacementChild.updateLocation(segment);
}
- assert child.parent == this;
- List<Node<Payload, PropertyPayload>> currentChildren =
childrenByName.get(childName);
- currentChildren.add(child);
- // Create a segment with the SNS ...
- Path.Segment segment = cache.pathFactory.createSegment(childName,
currentChildren.size());
- child.updateLocation(segment);
- // TODO: Can the location be different? If so, doesn't that mean
that the change requests
- // have to be updated???
}
return;
}
Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/AbstractJcrNode.java
===================================================================
--- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/AbstractJcrNode.java 2009-08-26 21:16:25
UTC (rev 1178)
+++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/AbstractJcrNode.java 2009-08-27 14:04:45
UTC (rev 1179)
@@ -1588,8 +1588,8 @@
if (destChildRelPath != null) {
Path destPath = pathFactory.create(destChildRelPath);
if (destPath.isAbsolute() || destPath.size() != 1) {
- throw new ItemNotFoundException(
-
JcrI18n.pathNotFound.text(destPath.getString(cache.context().getNamespaceRegistry()),
+ throw new
ItemNotFoundException(JcrI18n.pathNotFound.text(destPath.getString(cache.context()
+
.getNamespaceRegistry()),
cache.session().workspace().getName()));
}
@@ -1670,7 +1670,7 @@
* @see javax.jcr.Item#refresh(boolean)
*/
public void refresh( boolean keepChanges ) throws RepositoryException {
- this.cache.refresh(this.nodeId, keepChanges);
+ this.cache.refresh(this.nodeId, location.getPath(), keepChanges);
}
/**
Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/SessionCache.java
===================================================================
--- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/SessionCache.java 2009-08-26 21:16:25
UTC (rev 1178)
+++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/SessionCache.java 2009-08-27 14:04:45
UTC (rev 1179)
@@ -281,18 +281,22 @@
* </p>
*
* @param nodeId the identifier of the node that is to be saved; may not be null
+ * @param absolutePath the absolute path to the node; may not be null
* @param keepChanges indicates whether changed nodes should be kept or refreshed
from the repository.
* @throws InvalidItemStateException if the node being refreshed no longer exists
* @throws RepositoryException if any error resulting while saving the changes to the
repository
*/
public void refresh( NodeId nodeId,
+ Path absolutePath,
boolean keepChanges ) throws InvalidItemStateException,
RepositoryException {
assert nodeId != null;
try {
- Node<JcrNodePayload, JcrPropertyPayload> node =
graphSession.findNodeWith(nodeId);
+ Node<JcrNodePayload, JcrPropertyPayload> node =
graphSession.findNodeWith(nodeId, absolutePath);
graphSession.refresh(node, keepChanges);
} catch (InvalidStateException e) {
throw new InvalidItemStateException(e.getLocalizedMessage());
+ } catch (org.jboss.dna.graph.property.PathNotFoundException e) {
+ throw new InvalidItemStateException(e.getLocalizedMessage());
} catch (RepositorySourceException e) {
throw new RepositoryException(e.getLocalizedMessage());
}
Modified: trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java
===================================================================
--- trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java 2009-08-26 21:16:25 UTC
(rev 1178)
+++ trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java 2009-08-27 14:04:45 UTC
(rev 1179)
@@ -37,11 +37,13 @@
import org.apache.jackrabbit.test.api.NodeItemIsNewTest;
import org.apache.jackrabbit.test.api.NodeOrderableChildNodesTest;
import org.apache.jackrabbit.test.api.NodeRemoveMixinTest;
+import org.apache.jackrabbit.test.api.NodeTest;
import org.apache.jackrabbit.test.api.PropertyItemIsModifiedTest;
import org.apache.jackrabbit.test.api.PropertyItemIsNewTest;
import org.apache.jackrabbit.test.api.PropertyTest;
import org.apache.jackrabbit.test.api.RepositoryLoginTest;
import org.apache.jackrabbit.test.api.SerializationTest;
+import org.apache.jackrabbit.test.api.SessionTest;
import org.apache.jackrabbit.test.api.SetPropertyAssumeTypeTest;
import org.apache.jackrabbit.test.api.SetPropertyBooleanTest;
import org.apache.jackrabbit.test.api.SetPropertyCalendarTest;
@@ -190,9 +192,9 @@
addTestSuite(AddNodeTest.class);
addTestSuite(NamespaceRegistryTest.class);
// addTestSuite(ReferencesTest.class);
- // dna-466 addTestSuite(SessionTest.class);
+ addTestSuite(SessionTest.class);
// addTestSuite(SessionUUIDTest.class);
- // dna-466 addTestSuite(NodeTest.class);
+ addTestSuite(NodeTest.class);
// addTestSuite(NodeUUIDTest.class);
addTestSuite(NodeOrderableChildNodesTest.class);
addTestSuite(PropertyTest.class);