Fix AccumuloVertex.remove, and probably another bug with AccumuloEdge.remove

src/main/java/edu/jhuapl/tinkerpop/AccumuloEdge.java

@@ -15,7 +15,9 @@
 package edu.jhuapl.tinkerpop;
 
 import java.util.Map;
+import java.util.Set;
 
+import org.apache.commons.lang.ArrayUtils;
 import org.apache.log4j.Logger;
 
 import com.tinkerpop.blueprints.Direction;
@@ -75,9 +77,19 @@ public class AccumuloEdge extends AccumuloElement implements Edge {
     // Remove from named indexes.
     super.removeElementFromNamedIndexes();
 
-    // Remove from key/value indexes.
+    // If edge was removed already, forget it.
+    // This may happen due to self-loops...
+    if (!globals.getEdgeWrapper().elementExists(id)) {
+      return;
+    }
+
+    // Remove properties from key/value indexes.
+    Set<String> indexedKeys = globals.getIndexedKeysListWrapper()
+        .getIndexedKeys(Edge.class);
+
     Map<String, Object> props = globals.getEdgeWrapper()
-        .readProperties(this);
+        .readProperties(this, indexedKeys.toArray(ArrayUtils.EMPTY_STRING_ARRAY));
+
     for (String key : props.keySet()) {
       globals.getEdgeKeyIndexWrapper().removePropertyFromIndex(this,
           key, props.get(key));

src/main/java/edu/jhuapl/tinkerpop/AccumuloElement.java

@@ -18,6 +18,7 @@ import java.util.Set;
 
 import com.tinkerpop.blueprints.Element;
 import com.tinkerpop.blueprints.Index;
+import com.tinkerpop.blueprints.util.ExceptionFactory;
 import com.tinkerpop.blueprints.util.StringFactory;
 
 import edu.jhuapl.tinkerpop.cache.PropertyCache;

src/main/java/edu/jhuapl/tinkerpop/AccumuloGraph.java

@@ -14,10 +14,8 @@
  */
 package edu.jhuapl.tinkerpop;
 
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Iterator;
-import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
@@ -123,6 +121,7 @@ public class AccumuloGraph implements Graph, KeyIndexableGraph, IndexableGraph {
    * @deprecated This will go away along with {@link #vertexBW}.
    * @throws Exception
    */
+  @Deprecated
   private void setupWriters() throws Exception {
     vertexBW = globals.getMtbw().getBatchWriter(globals.getConfig().getVertexTableName());
   }
@@ -302,79 +301,7 @@ public class AccumuloGraph implements Graph, KeyIndexableGraph, IndexableGraph {
 
   @Override
   public void removeVertex(Vertex vertex) {
-    globals.getCaches().remove(vertex.getId(), Vertex.class);
-
-    if (!globals.getConfig().getIndexableGraphDisabled()) {
-      removeElementFromNamedIndexes(vertex);
-    }
-
-    Scanner scan = getElementScanner(Vertex.class);
-    scan.setRange(new Range(vertex.getId().toString()));
-
-    BatchDeleter edgedeleter = null;
-    BatchDeleter vertexdeleter = null;
-    BatchWriter indexdeleter = getVertexIndexWriter();
-    try {
-      // Set up Deleters
-      edgedeleter = globals.getConfig().getConnector().createBatchDeleter(globals.getConfig().getEdgeTableName(),
-          globals.getConfig().getAuthorizations(), globals.getConfig().getQueryThreads(),
-          globals.getConfig().getBatchWriterConfig());
-      vertexdeleter = globals.getConfig().getConnector().createBatchDeleter(globals.getConfig().getVertexTableName(),
-          globals.getConfig().getAuthorizations(), globals.getConfig().getQueryThreads(),
-          globals.getConfig().getBatchWriterConfig());
-    } catch (Exception e) {
-      throw new AccumuloGraphException(e);
-    }
-    Iterator<Entry<Key,Value>> iter = scan.iterator();
-    List<Range> ranges = new ArrayList<Range>();
-    if (!iter.hasNext()) {
-      throw ExceptionFactory.vertexWithIdDoesNotExist(vertex.getId());
-    }
-    try {
-      // Search for edges
-      while (iter.hasNext()) {
-        Entry<Key,Value> e = iter.next();
-        Key k = e.getKey();
-
-        if (k.getColumnFamily().toString().equals(Constants.OUT_EDGE) ||
-            k.getColumnFamily().toString().equals(Constants.IN_EDGE)) {
-          ranges.add(new Range(k.getColumnQualifier().toString().split(Constants.ID_DELIM)[1]));
-
-          Mutation vm = new Mutation(k.getColumnQualifier().toString().split(Constants.ID_DELIM)[0]);
-          vm.putDelete(invert(k.getColumnFamily()),
-              new Text(vertex.getId().toString() + Constants.ID_DELIM + k.getColumnQualifier()
-                  .toString().split(Constants.ID_DELIM)[1]));
-          vertexBW.addMutation(vm);
-        } else {
-          Mutation m = new Mutation(e.getValue().get());
-          m.putDelete(k.getColumnFamily(), k.getRow());
-          indexdeleter.addMutation(m);
-        }
-
-      }
-      globals.checkedFlush();
-      scan.close();
-
-      // If Edges are found, delete the whole row
-      if (!ranges.isEmpty()) {
-        // TODO don't we also have to propagate these deletes to the
-        // vertex index table?
-        edgedeleter.setRanges(ranges);
-        edgedeleter.delete();
-        ranges.clear();
-      }
-      // Delete the whole vertex row
-      ranges.add(new Range(vertex.getId().toString()));
-      vertexdeleter.setRanges(ranges);
-      vertexdeleter.delete();
-    } catch (Exception e) {
-      throw new AccumuloGraphException(e);
-    } finally {
-      if (edgedeleter != null)
-        edgedeleter.close();
-      if (vertexdeleter != null)
-        vertexdeleter.close();
-    }
+    vertex.remove();
   }
 
   // Maybe an Custom Iterator could make this better.
@@ -382,6 +309,7 @@ public class AccumuloGraph implements Graph, KeyIndexableGraph, IndexableGraph {
    * @deprecated Move to appropriate location.
    * @param element
    */
+  @Deprecated
   private void removeElementFromNamedIndexes(Element element) {
     for (Index<? extends Element> index : getIndices()) {
       ((AccumuloIndex<? extends Element>) index).getWrapper().removeElementFromIndex(element);

src/main/java/edu/jhuapl/tinkerpop/AccumuloVertex.java

@@ -14,11 +14,17 @@
  */
 package edu.jhuapl.tinkerpop;
 
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.commons.lang.ArrayUtils;
+
 import com.tinkerpop.blueprints.Direction;
 import com.tinkerpop.blueprints.Edge;
 import com.tinkerpop.blueprints.Vertex;
 import com.tinkerpop.blueprints.VertexQuery;
 import com.tinkerpop.blueprints.util.DefaultVertexQuery;
+import com.tinkerpop.blueprints.util.ExceptionFactory;
 
 /**
  * TODO
@@ -55,6 +61,23 @@ public class AccumuloVertex extends AccumuloElement implements Vertex {
 
     super.removeElementFromNamedIndexes();
 
+    // Throw exception if the element does not exist.
+    if (!globals.getVertexWrapper().elementExists(id)) {
+      throw ExceptionFactory.vertexWithIdDoesNotExist(getId());
+    }
+
+    // Remove properties from key/value indexes.
+    Set<String> indexedKeys = globals.getIndexedKeysListWrapper()
+        .getIndexedKeys(Vertex.class);
+
+    Map<String, Object> props = globals.getVertexWrapper()
+        .readProperties(this, indexedKeys.toArray(ArrayUtils.EMPTY_STRING_ARRAY));
+
+    for (String key : props.keySet()) {
+      globals.getVertexKeyIndexWrapper().removePropertyFromIndex(this,
+          key, props.get(key));
+    }
+
     // Remove edges incident to this vertex.
     for (Edge edge : getEdges(Direction.BOTH)) {
       edge.remove();

src/main/java/edu/jhuapl/tinkerpop/parser/PropertyParser.java

@@ -40,7 +40,7 @@ public class PropertyParser implements EntryParser<Map<String, Object>> {
 
       Key key = entry.getKey();
 
-      if (!isExistenceKey(key)) {
+      if (!isMetaKey(key)) {
         String attr = key.getColumnFamily().toString();
         Object value = AccumuloByteSerializer.deserialize(entry.getValue().get());
         props.put(attr, value);
@@ -51,14 +51,19 @@ public class PropertyParser implements EntryParser<Map<String, Object>> {
   }
 
   /**
-   * Test whether the given Accumulo key represents an
-   * element's existence (i.e. not a property).
+   * Test whether the given Accumulo key represents a
+   * metadata key (e.g. existence, edge endpoint, etc),
+   * rather than a property.
    * @param key
    * @return
    */
-  private static boolean isExistenceKey(Key key) {
-    return Constants.LABEL.equals(key.getColumnFamily().toString()) &&
-        Constants.EXISTS.equals(key.getColumnQualifier().toString());
+  private static boolean isMetaKey(Key key) {
+    String cf = key.getColumnFamily().toString();
+    String cq = key.getColumnQualifier().toString();
+    return (Constants.LABEL.equals(cf) &&
+        Constants.EXISTS.equals(cq)) ||
+        Constants.IN_EDGE.equals(cf) ||
+        Constants.OUT_EDGE.equals(cf);
   }
 
 }

src/main/java/edu/jhuapl/tinkerpop/tables/core/ElementTableWrapper.java

@@ -112,6 +112,26 @@ public abstract class ElementTableWrapper extends BaseTableWrapper {
     return props;
   }
 
+  /**
+   * Return true if the element with given id exists.
+   * @param id
+   * @return
+   */
+  public boolean elementExists(String id) {
+    Scanner scan = null;
+    try {
+      scan = getScanner();
+      scan.setRange(Range.exact(id));
+      scan.fetchColumnFamily(new Text(Constants.LABEL));
+      return new PropertyParser().parse(scan) != null;
+
+    } finally {
+      if (scan != null) {
+        scan.close();
+      }
+    }
+  }
+
   /**
    * Get all property keys for the given element id.
    * @param id