SOLR-13037: Harden TestSimGenericDistributedQueue

apache · Dec 13, 2018 · b072a7d · b072a7d
1 parent 3d6a09e
commit b072a7d
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 33 deletions.
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimDistribStateManager.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimDistribStateManager.java
@@ -311,17 +311,7 @@ private Node traverse(String path, boolean create, CreateMode mode) throws IOExc
       n = parentNode.children != null ? parentNode.children.get(currentName) : null;
       if (n == null) {
         if (create) {
-          if ((parentNode.mode == CreateMode.EPHEMERAL || parentNode.mode == CreateMode.EPHEMERAL_SEQUENTIAL) &&
-              (mode == CreateMode.EPHEMERAL || mode == CreateMode.EPHEMERAL_SEQUENTIAL)) {
-            throw new IOException("NoChildrenEphemerals for " + parentNode.path);
-          }
-          if (CreateMode.PERSISTENT_SEQUENTIAL == mode || CreateMode.EPHEMERAL_SEQUENTIAL == mode) {
-            currentName = currentName + String.format(Locale.ROOT, "%010d", parentNode.seq);
-            parentNode.seq++;
-          }
-          currentPath.append(currentName);
-          n = new Node(parentNode, currentName, currentPath.toString(), mode, id);
-          parentNode.setChild(currentName, n);
+          n = createNode(parentNode, mode, currentPath, currentName,true);
         } else {
           break;
         }
@@ -333,6 +323,26 @@ private Node traverse(String path, boolean create, CreateMode mode) throws IOExc
     return n;
   }
 
+  private Node createNode(Node parentNode, CreateMode mode, StringBuilder fullChildPath, String baseChildName, boolean attachToParent) throws IOException {
+    String nodeName = baseChildName;
+    if ((parentNode.mode == CreateMode.EPHEMERAL || parentNode.mode == CreateMode.EPHEMERAL_SEQUENTIAL) &&
+        (mode == CreateMode.EPHEMERAL || mode == CreateMode.EPHEMERAL_SEQUENTIAL)) {
+      throw new IOException("NoChildrenEphemerals for " + parentNode.path);
+    }
+    if (CreateMode.PERSISTENT_SEQUENTIAL == mode || CreateMode.EPHEMERAL_SEQUENTIAL == mode) {
+      nodeName = nodeName + String.format(Locale.ROOT, "%010d", parentNode.seq);
+      parentNode.seq++;
+    }
+
+    fullChildPath.append(nodeName);
+    Node child = new Node(parentNode, nodeName, fullChildPath.toString(), mode, id);
+
+    if (attachToParent) {
+      parentNode.setChild(nodeName, child);
+    }
+    return child;
+  }
+
   @Override
   public void close() throws IOException {
     multiLock.lock();
@@ -444,33 +454,39 @@ public String createData(String path, byte[] data, CreateMode mode) throws Alrea
     if ((CreateMode.EPHEMERAL == mode || CreateMode.PERSISTENT == mode) && hasData(path)) {
       throw new AlreadyExistsException(path);
     }
-    // check if parent exists
+
     String relPath = path.charAt(0) == '/' ? path.substring(1) : path;
-    if (relPath.length() > 0) { // non-root path - check if parent exists
-      String[] elements = relPath.split("/");
-      StringBuilder sb = new StringBuilder();
-      for (int i = 0; i < elements.length - 1; i++) {
-        sb.append('/');
-        sb.append(elements[i]);
-      }
-      if (!hasData(sb.toString())) {
-        throw new NoSuchElementException(sb.toString());
-      }
+    if (relPath.length() == 0) {
+      // TODO should trying to create a root node throw an exception since its always init'd in the ctor?
+      return null;
     }
-    Node n = null;
-    multiLock.lock();
-    try {
-      n = traverse(path, true, mode);
-    } finally {
-      multiLock.unlock();
+
+    // non-root-node.  Make sure parent exists.
+    String[] elements = relPath.split("/");
+    StringBuilder parentStringBuilder = new StringBuilder();
+    for (int i = 0; i < elements.length - 1; i++) {
+      parentStringBuilder.append('/');
+      parentStringBuilder.append(elements[i]);
+    }
+    if (!hasData(parentStringBuilder.toString())) {
+      throw new NoSuchElementException(parentStringBuilder.toString());
     }
+
+    multiLock.lock();
     try {
-      n.setData(data, -1);
-      return n.path;
+      String nodeName = elements[elements.length-1];
+      Node parentNode = traverse(parentStringBuilder.toString(), false, mode);
+      Node childNode = createNode(parentNode, mode, parentStringBuilder.append("/"), nodeName, false);
+      childNode.setData(data, -1);
+      parentNode.setChild(childNode.name, childNode);
+      return childNode.path;
     } catch (BadVersionException e) {
       // not happening
       return null;
+    } finally {
+      multiLock.unlock();
     }
+
   }
 
   @Override

diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimGenericDistributedQueue.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimGenericDistributedQueue.java
@@ -16,15 +16,12 @@
  */
 package org.apache.solr.cloud.autoscaling.sim;
 
-import org.apache.lucene.util.LuceneTestCase.AwaitsFix;
-
 import org.apache.solr.client.solrj.cloud.DistributedQueue;
 import org.apache.solr.client.solrj.cloud.DistribStateManager;
 
 /**
  *
  */
-@AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/SOLR-13037")
 public class TestSimGenericDistributedQueue extends TestSimDistributedQueue {
   DistribStateManager stateManager = new SimDistribStateManager();