You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by ra...@apache.org on 2017/02/10 17:26:53 UTC
zookeeper git commit: ZOOKEEPER-2680: Correct DataNode.getChildren()
inconsistent behaviour
Repository: zookeeper
Updated Branches:
refs/heads/branch-3.5 4bc78ee81 -> 91c400829
ZOOKEEPER-2680: Correct DataNode.getChildren() inconsistent behaviour
Author: Mohammad Arshad <ar...@apache.org>
Reviewers: Edward Ribeiro <ed...@gmail.com>, Abraham Fine <af...@apache.org>, Michael Han <ha...@apache.org>, Rakesh Radhakrishnan <ra...@apache.org>
Closes #161 from arshadmohammad/ZOOKEEPER-2680-br-3.5
Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo
Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/91c40082
Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/91c40082
Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/91c40082
Branch: refs/heads/branch-3.5
Commit: 91c400829ef92110d17ca1f328dda4749aae57b0
Parents: 4bc78ee
Author: Mohammad Arshad <ar...@apache.org>
Authored: Sat Feb 11 04:37:29 2017 +0530
Committer: Rakesh Radhakrishnan <ra...@apache.org>
Committed: Sat Feb 11 04:37:29 2017 +0530
----------------------------------------------------------------------
.../org/apache/zookeeper/server/DataNode.java | 7 ++-
.../org/apache/zookeeper/server/DataTree.java | 43 ++++---------
.../zookeeper/server/PrepRequestProcessor.java | 3 +-
.../zookeeper/server/SnapshotFormatter.java | 6 +-
.../apache/zookeeper/server/DataNodeTest.java | 65 ++++++++++++++++++++
5 files changed, 86 insertions(+), 38 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/zookeeper/blob/91c40082/src/java/main/org/apache/zookeeper/server/DataNode.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/DataNode.java b/src/java/main/org/apache/zookeeper/server/DataNode.java
index 40cc8ea..1bad238 100644
--- a/src/java/main/org/apache/zookeeper/server/DataNode.java
+++ b/src/java/main/org/apache/zookeeper/server/DataNode.java
@@ -57,6 +57,8 @@ public class DataNode implements Record {
*/
private Set<String> children = null;
+ private static final Set<String> EMPTY_SET = Collections.emptySet();
+
/**
* default constructor for the datanode
*/
@@ -122,11 +124,12 @@ public class DataNode implements Record {
/**
* convenience methods to get the children
*
- * @return the children of this datanode
+ * @return the children of this datanode. If the datanode has no children, empty
+ * set is returned
*/
public synchronized Set<String> getChildren() {
if (children == null) {
- return children;
+ return EMPTY_SET;
}
return Collections.unmodifiableSet(children);
http://git-wip-us.apache.org/repos/asf/zookeeper/blob/91c40082/src/java/main/org/apache/zookeeper/server/DataTree.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/DataTree.java b/src/java/main/org/apache/zookeeper/server/DataTree.java
index 57eb952..7bd3380 100644
--- a/src/java/main/org/apache/zookeeper/server/DataTree.java
+++ b/src/java/main/org/apache/zookeeper/server/DataTree.java
@@ -449,7 +449,7 @@ public class DataTree {
}
synchronized (parent) {
Set<String> children = parent.getChildren();
- if (children != null && children.contains(childName)) {
+ if (children.contains(childName)) {
throw new KeeperException.NodeExistsException();
}
@@ -662,13 +662,7 @@ public class DataTree {
if (stat != null) {
n.copyStat(stat);
}
- ArrayList<String> children;
- Set<String> childs = n.getChildren();
- if (childs == null) {
- children = new ArrayList<String>(0);
- } else {
- children = new ArrayList<String>(childs);
- }
+ List<String> children=new ArrayList<String>(n.getChildren());
if (watcher != null) {
childWatches.addWatch(path, watcher);
@@ -1025,17 +1019,12 @@ public class DataTree {
int len = 0;
synchronized (node) {
Set<String> childs = node.getChildren();
- if (childs != null) {
- children = childs.toArray(new String[childs.size()]);
- }
+ children = childs.toArray(new String[childs.size()]);
len = (node.data == null ? 0 : node.data.length);
}
// add itself
counts.count += 1;
counts.bytes += len;
- if (children == null || children.length == 0) {
- return;
- }
for (String child : children) {
getCounts(path + "/" + child, counts);
}
@@ -1075,11 +1064,9 @@ public class DataTree {
String children[] = null;
synchronized (node) {
Set<String> childs = node.getChildren();
- if (childs != null) {
- children = childs.toArray(new String[childs.size()]);
- }
+ children = childs.toArray(new String[childs.size()]);
}
- if (children == null || children.length == 0) {
+ if (children.length == 0) {
// this node does not have a child
// is the leaf node
// check if its the leaf node
@@ -1138,23 +1125,19 @@ public class DataTree {
//are never changed
nodeCopy = new DataNode(node.data, node.acl, statCopy);
Set<String> childs = node.getChildren();
- if (childs != null) {
- children = childs.toArray(new String[childs.size()]);
- }
+ children = childs.toArray(new String[childs.size()]);
}
oa.writeString(pathString, "path");
oa.writeRecord(nodeCopy, "node");
path.append('/');
int off = path.length();
- if (children != null) {
- for (String child : children) {
- // since this is single buffer being resused
- // we need
- // to truncate the previous bytes of string.
- path.delete(off, Integer.MAX_VALUE);
- path.append(child);
- serializeNode(oa, path);
- }
+ for (String child : children) {
+ // since this is single buffer being resused
+ // we need
+ // to truncate the previous bytes of string.
+ path.delete(off, Integer.MAX_VALUE);
+ path.append(child);
+ serializeNode(oa, path);
}
}
http://git-wip-us.apache.org/repos/asf/zookeeper/blob/91c40082/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java b/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
index b02ff38..4899c81 100644
--- a/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
+++ b/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
@@ -165,8 +165,7 @@ public class PrepRequestProcessor extends ZooKeeperCriticalThread implements
synchronized(n) {
children = n.getChildren();
}
- lastChange = new ChangeRecord(-1, path, n.stat,
- children != null ? children.size() : 0,
+ lastChange = new ChangeRecord(-1, path, n.stat, children.size(),
zks.getZKDatabase().aclForNode(n));
}
}
http://git-wip-us.apache.org/repos/asf/zookeeper/blob/91c40082/src/java/main/org/apache/zookeeper/server/SnapshotFormatter.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/SnapshotFormatter.java b/src/java/main/org/apache/zookeeper/server/SnapshotFormatter.java
index f94c54d..bc43402 100644
--- a/src/java/main/org/apache/zookeeper/server/SnapshotFormatter.java
+++ b/src/java/main/org/apache/zookeeper/server/SnapshotFormatter.java
@@ -94,10 +94,8 @@ public class SnapshotFormatter {
}
children = n.getChildren();
}
- if (children != null) {
- for (String child : children) {
- printZnode(dataTree, name + (name.equals("/") ? "" : "/") + child);
- }
+ for (String child : children) {
+ printZnode(dataTree, name + (name.equals("/") ? "" : "/") + child);
}
}
http://git-wip-us.apache.org/repos/asf/zookeeper/blob/91c40082/src/java/test/org/apache/zookeeper/server/DataNodeTest.java
----------------------------------------------------------------------
diff --git a/src/java/test/org/apache/zookeeper/server/DataNodeTest.java b/src/java/test/org/apache/zookeeper/server/DataNodeTest.java
new file mode 100644
index 0000000..6289766
--- /dev/null
+++ b/src/java/test/org/apache/zookeeper/server/DataNodeTest.java
@@ -0,0 +1,65 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zookeeper.server;
+
+import static org.junit.Assert.*;
+
+import java.util.Set;
+
+import org.junit.Test;
+
+public class DataNodeTest {
+
+ @Test
+ public void testGetChildrenShouldReturnEmptySetWhenThereAreNoChidren() {
+ // create DataNode and call getChildren
+ DataNode dataNode = new DataNode();
+ Set<String> children = dataNode.getChildren();
+ assertNotNull(children);
+ assertEquals(0, children.size());
+
+ // add child,remove child and then call getChildren
+ String child = "child";
+ dataNode.addChild(child);
+ dataNode.removeChild(child);
+ children = dataNode.getChildren();
+ assertNotNull(children);
+ assertEquals(0, children.size());
+
+ // Returned empty set must not be modifiable
+ children = dataNode.getChildren();
+ try {
+ children.add("new child");
+ fail("UnsupportedOperationException is expected");
+ } catch (UnsupportedOperationException e) {
+ // do nothing
+ }
+ }
+
+ @Test
+ public void testGetChildrenReturnsImmutableEmptySet() {
+ DataNode dataNode = new DataNode();
+ Set<String> children = dataNode.getChildren();
+ try {
+ children.add("new child");
+ fail("UnsupportedOperationException is expected");
+ } catch (UnsupportedOperationException e) {
+ // do nothing
+ }
+ }
+}