Property cache updates

Added property caching unit test
Bug fix in property caching
Cruft cleanup and deprecate AccumuloGraph methods

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

@@ -86,8 +86,12 @@ public abstract class AccumuloElement implements Element {
   @Override
   public <T> T removeProperty(String key) {
     makeCache();
+    T old = globals.getGraph().removeProperty(type, this, key);
+    // MDL 31 Dec 2014:  AccumuloGraph.removeProperty
+    //   calls getProperty which populates the cache.
+    //   So the order here is important (for now).
     propertyCache.remove(key);
-    return globals.getGraph().removeProperty(type, this, key);
+    return old;
   }
 
   @Override
@@ -113,6 +117,14 @@ public abstract class AccumuloElement implements Element {
     return getClass().hashCode() ^ id.hashCode();
   }
 
+  /*
+   * Internal method for unit tests.
+   * @return
+   */
+  PropertyCache getPropertyCache() {
+    return propertyCache;
+  }
+
   /**
    * @deprecated This is used in {@link AccumuloGraph} but needs to go away.
    * @param key

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

@@ -834,7 +834,7 @@ public class AccumuloGraph implements Graph, KeyIndexableGraph, IndexableGraph {
     try {
       writer.flush();
     } catch (MutationsRejectedException e) {
-      e.printStackTrace();
+      throw new AccumuloGraphException(e);
     }
   }
 
@@ -847,41 +847,12 @@ public class AccumuloGraph implements Graph, KeyIndexableGraph, IndexableGraph {
   // methods used by AccumuloElement, AccumuloVertex, AccumuloEdge to interact
   // with the backing Accumulo data store...
 
-  void preloadProperties(AccumuloElement element, Class<? extends Element> type) {
-    String[] toPreload = config.getPreloadedProperties();
-    if (toPreload == null) {
-      return;
-    }
-
-    Scanner s = getElementScanner(type);
-    s.setRange(new Range(element.getId().toString()));
-
-    // user has requested specific properties...
-    Text colf = new Text("");
-    for (String key : toPreload) {
-      if (StringFactory.LABEL.equals(key)) {
-        colf.set(AccumuloGraph.LABEL);
-      } else {
-        colf.set(key);
-      }
-      s.fetchColumnFamily(colf);
-    }
-
-    Iterator<Entry<Key, Value>> iter = s.iterator();
-    // Integer timeout = config.getPropertyCacheTimeoutMillis(); // Change this
-    while (iter.hasNext()) {
-      Entry<Key,Value> entry = iter.next();
-      Object val = AccumuloByteSerializer.deserialize(entry.getValue().get());
-      element.cacheProperty(entry.getKey().getColumnFamily().toString(), val);
-    }
-    s.close();
-  }
-
   /**
    * Sets the property. Requires a round-trip to Accumulo to see if the property exists
    * iff the provided key has an index. Therefore, for best performance, if at
    * all possible, create indices after bulk ingest.
    * 
+   * @deprecated Move to appropriate place
    * @param type
    * @param id
    * @param key
@@ -926,6 +897,13 @@ public class AccumuloGraph implements Graph, KeyIndexableGraph, IndexableGraph {
     return getVertexIndexWriter();
   }
 
+  /**
+   * @deprecated Move to appropriate place
+   * @param type
+   * @param element
+   * @param key
+   * @return
+   */
   <T> T removeProperty(Class<? extends Element> type, Element element, String key) {
     if (StringFactory.LABEL.equals(key) || SLABEL.equals(key)) {
       throw new AccumuloGraphException("Cannot remove the " + StringFactory.LABEL + " property.");
@@ -948,6 +926,12 @@ public class AccumuloGraph implements Graph, KeyIndexableGraph, IndexableGraph {
     return obj;
   }
 
+  /**
+   * @deprecated Move to appropriate place
+   * @param edgeId
+   * @param direction
+   * @return
+   */
   Vertex getEdgeVertex(String edgeId, Direction direction) {
     Scanner s = getElementScanner(Edge.class);
     try {
@@ -998,7 +982,7 @@ public class AccumuloGraph implements Graph, KeyIndexableGraph, IndexableGraph {
 
   @Override
   public String toString() {
-    return "accumulograph";
+    return AccumuloGraphConfiguration.ACCUMULO_GRAPH_CLASS.getSimpleName().toLowerCase();
   }
 
   @Override

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

@@ -43,6 +43,7 @@ import org.apache.commons.configuration.Configuration;
 import org.apache.commons.configuration.PropertiesConfiguration;
 import org.apache.hadoop.io.Text;
 
+import com.tinkerpop.blueprints.Graph;
 import com.tinkerpop.blueprints.IndexableGraph;
 import com.tinkerpop.blueprints.KeyIndexableGraph;
 
@@ -69,6 +70,11 @@ implements Serializable {
   private MiniAccumuloCluster accumuloMiniCluster;
 
 
+  /**
+   * The {@link AccumuloGraph} class.
+   */
+  public static final Class<? extends Graph> ACCUMULO_GRAPH_CLASS = AccumuloGraph.class;
+
   /**
    * The fully-qualified class name of the class that implements
    * the TinkerPop Graph interface.
@@ -76,7 +82,7 @@ implements Serializable {
    * which type to instantiate.
    */
   public static final String ACCUMULO_GRAPH_CLASSNAME =
-      AccumuloGraph.class.getCanonicalName();
+      ACCUMULO_GRAPH_CLASS.getCanonicalName();
 
   /**
    * An enumeration used by {@link AccumuloGraphConfiguration#setInstanceType(InstanceType)}

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

@@ -13,6 +13,7 @@ package edu.jhuapl.tinkerpop;
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Set;
 
 /**
  * Cache for storing element properties.
@@ -31,6 +32,14 @@ public class PropertyCache {
     this.values = new HashMap<String, TimedValue>();
   }
 
+  public boolean containsKey(String key) {
+    return values.containsKey(key);
+  }
+
+  public Set<String> keySet() {
+    return values.keySet();
+  }
+
   public void put(String key, Object value) {
     Integer timeout = getTimeout(key);
 
@@ -74,6 +83,11 @@ public class PropertyCache {
     values.clear();
   }
 
+  @Override
+  public String toString() {
+    return values.toString();
+  }
+
   /**
    * Return the timeout for the given key.
    * Checks for a key-specific timeout
@@ -109,5 +123,10 @@ public class PropertyCache {
     public Long getExpiry() {
       return expiry;
     }
+
+    @Override
+    public String toString() {
+      return "[" + value + ", " + expiry + "]";
+    }
   }
 }

src/test/java/edu/jhuapl/tinkerpop/ElementPropertyCachingTest.java

@@ -0,0 +1,229 @@
+/******************************************************************************
+ *                              COPYRIGHT NOTICE                              *
+ * Copyright (c) 2014 The Johns Hopkins University/Applied Physics Laboratory *
+ *                            All rights reserved.                            *
+ *                                                                            *
+ * This material may only be used, modified, or reproduced by or for the      *
+ * U.S. Government pursuant to the license rights granted under FAR clause    *
+ * 52.227-14 or DFARS clauses 252.227-7013/7014.                              *
+ *                                                                            *
+ * For any other permissions, please contact the Legal Office at JHU/APL.     *
+ ******************************************************************************/
+package edu.jhuapl.tinkerpop;
+
+import static org.junit.Assert.*;
+
+import org.junit.Test;
+
+import com.google.common.collect.Sets;
+import com.tinkerpop.blueprints.Element;
+import com.tinkerpop.blueprints.Graph;
+import com.tinkerpop.blueprints.GraphFactory;
+
+/**
+ * Tests related to {@link Element}-based property caching.
+ */
+public class ElementPropertyCachingTest {
+
+  private static final int TIMEOUT = 300000;
+  private static final String NON_CACHED = "noncached";
+  private static final String CACHED = "cached";
+
+  @Test
+  public void testCachingDisabled() {
+    AccumuloGraphConfiguration cfg =
+        AccumuloGraphTestUtils.generateGraphConfig("cachingDisabled");
+    assertTrue(cfg.getPropertyCacheTimeout(null) <= 0);
+    assertTrue(cfg.getPropertyCacheTimeout(NON_CACHED) <= 0);
+    assertTrue(cfg.getPropertyCacheTimeout(CACHED) <= 0);
+
+    Graph graph = open(cfg);
+    load(graph);
+
+    AccumuloVertex a = (AccumuloVertex) graph.getVertex("A");
+    AccumuloVertex b = (AccumuloVertex) graph.getVertex("B");
+    AccumuloVertex c = (AccumuloVertex) graph.getVertex("C");
+
+    assertEquals(null, a.getProperty(NON_CACHED));
+    assertEquals(true, b.getProperty(NON_CACHED));
+    assertEquals(null, c.getProperty(NON_CACHED));
+    assertEquals(null, a.getProperty(CACHED));
+    assertEquals(null, b.getProperty(CACHED));
+    assertEquals(true, c.getProperty(CACHED));
+
+    assertEquals(null, a.getPropertyCache().get(NON_CACHED));
+    assertEquals(null, b.getPropertyCache().get(NON_CACHED));
+    assertEquals(null, c.getPropertyCache().get(NON_CACHED));
+    assertEquals(null, a.getPropertyCache().get(CACHED));
+    assertEquals(null, b.getPropertyCache().get(CACHED));
+    assertEquals(null, c.getPropertyCache().get(CACHED));
+
+    assertEquals(Sets.newHashSet(), a.getPropertyCache().keySet());
+    assertEquals(Sets.newHashSet(), b.getPropertyCache().keySet());
+    assertEquals(Sets.newHashSet(), c.getPropertyCache().keySet());
+
+    a.removeProperty(NON_CACHED);
+    b.removeProperty(NON_CACHED);
+    c.removeProperty(NON_CACHED);
+    a.removeProperty(CACHED);
+    b.removeProperty(CACHED);
+    c.removeProperty(CACHED);
+
+    assertEquals(null, a.getProperty(NON_CACHED));
+    assertEquals(null, b.getProperty(NON_CACHED));
+    assertEquals(null, c.getProperty(NON_CACHED));
+    assertEquals(null, a.getProperty(CACHED));
+    assertEquals(null, b.getProperty(CACHED));
+    assertEquals(null, c.getProperty(CACHED));
+
+    assertEquals(null, a.getPropertyCache().get(NON_CACHED));
+    assertEquals(null, b.getPropertyCache().get(NON_CACHED));
+    assertEquals(null, c.getPropertyCache().get(NON_CACHED));
+    assertEquals(null, a.getPropertyCache().get(CACHED));
+    assertEquals(null, b.getPropertyCache().get(CACHED));
+    assertEquals(null, c.getPropertyCache().get(CACHED));
+
+    assertEquals(Sets.newHashSet(), a.getPropertyCache().keySet());
+    assertEquals(Sets.newHashSet(), b.getPropertyCache().keySet());
+    assertEquals(Sets.newHashSet(), c.getPropertyCache().keySet());
+
+    graph.shutdown();
+  }
+
+  @Test
+  public void testSpecificCaching() {
+    AccumuloGraphConfiguration cfg =
+        AccumuloGraphTestUtils.generateGraphConfig("getProperty");
+    cfg.setPropertyCacheTimeout(CACHED, TIMEOUT);
+
+    assertTrue(cfg.getPropertyCacheTimeout(null) <= 0);
+    assertTrue(cfg.getPropertyCacheTimeout(NON_CACHED) <= 0);
+    assertEquals(TIMEOUT, cfg.getPropertyCacheTimeout(CACHED));
+
+    Graph graph = open(cfg);
+    load(graph);
+
+    AccumuloVertex a = (AccumuloVertex) graph.getVertex("A");
+    AccumuloVertex b = (AccumuloVertex) graph.getVertex("B");
+    AccumuloVertex c = (AccumuloVertex) graph.getVertex("C");
+
+    assertEquals(null, a.getProperty(NON_CACHED));
+    assertEquals(true, b.getProperty(NON_CACHED));
+    assertEquals(null, c.getProperty(NON_CACHED));
+    assertEquals(null, a.getProperty(CACHED));
+    assertEquals(null, b.getProperty(CACHED));
+    assertEquals(true, c.getProperty(CACHED));
+
+    assertEquals(null, a.getPropertyCache().get(NON_CACHED));
+    assertEquals(null, b.getPropertyCache().get(NON_CACHED));
+    assertEquals(null, c.getPropertyCache().get(NON_CACHED));
+    assertEquals(null, a.getPropertyCache().get(CACHED));
+    assertEquals(null, b.getPropertyCache().get(CACHED));
+    assertEquals(true, c.getPropertyCache().get(CACHED));
+
+    assertEquals(Sets.newHashSet(), a.getPropertyCache().keySet());
+    assertEquals(Sets.newHashSet(), b.getPropertyCache().keySet());
+    assertEquals(Sets.newHashSet(CACHED), c.getPropertyCache().keySet());
+
+    a.removeProperty(NON_CACHED);
+    b.removeProperty(NON_CACHED);
+    c.removeProperty(NON_CACHED);
+    a.removeProperty(CACHED);
+    b.removeProperty(CACHED);
+    c.removeProperty(CACHED);
+
+    assertEquals(null, a.getProperty(NON_CACHED));
+    assertEquals(null, b.getProperty(NON_CACHED));
+    assertEquals(null, c.getProperty(NON_CACHED));
+    assertEquals(null, a.getProperty(CACHED));
+    assertEquals(null, b.getProperty(CACHED));
+    assertEquals(null, c.getProperty(CACHED));
+
+    assertEquals(null, a.getPropertyCache().get(NON_CACHED));
+    assertEquals(null, b.getPropertyCache().get(NON_CACHED));
+    assertEquals(null, c.getPropertyCache().get(NON_CACHED));
+    assertEquals(null, a.getPropertyCache().get(CACHED));
+    assertEquals(null, b.getPropertyCache().get(CACHED));
+    assertEquals(null, c.getPropertyCache().get(CACHED));
+
+    assertEquals(Sets.newHashSet(), a.getPropertyCache().keySet());
+    assertEquals(Sets.newHashSet(), b.getPropertyCache().keySet());
+    assertEquals(Sets.newHashSet(), c.getPropertyCache().keySet());
+
+    graph.shutdown();
+  }
+
+  @Test
+  public void testAllCaching() {
+    AccumuloGraphConfiguration cfg =
+        AccumuloGraphTestUtils.generateGraphConfig("setProperty");
+    cfg.setPropertyCacheTimeout(null, TIMEOUT);
+    cfg.setPropertyCacheTimeout(CACHED, TIMEOUT);
+
+    assertEquals(TIMEOUT, cfg.getPropertyCacheTimeout(null));
+    assertEquals(TIMEOUT, cfg.getPropertyCacheTimeout(NON_CACHED));
+    assertEquals(TIMEOUT, cfg.getPropertyCacheTimeout(CACHED));
+
+    Graph graph = open(cfg);
+    load(graph);
+
+    AccumuloVertex a = (AccumuloVertex) graph.getVertex("A");
+    AccumuloVertex b = (AccumuloVertex) graph.getVertex("B");
+    AccumuloVertex c = (AccumuloVertex) graph.getVertex("C");
+
+    assertEquals(null, a.getProperty(NON_CACHED));
+    assertEquals(true, b.getProperty(NON_CACHED));
+    assertEquals(null, c.getProperty(NON_CACHED));
+    assertEquals(null, a.getProperty(CACHED));
+    assertEquals(null, b.getProperty(CACHED));
+    assertEquals(true, c.getProperty(CACHED));
+
+    assertEquals(null, a.getPropertyCache().get(NON_CACHED));
+    assertEquals(true, b.getPropertyCache().get(NON_CACHED));
+    assertEquals(null, c.getPropertyCache().get(NON_CACHED));
+    assertEquals(null, a.getPropertyCache().get(CACHED));
+    assertEquals(null, b.getPropertyCache().get(CACHED));
+    assertEquals(true, c.getPropertyCache().get(CACHED));
+
+    assertEquals(Sets.newHashSet(), a.getPropertyCache().keySet());
+    assertEquals(Sets.newHashSet(NON_CACHED), b.getPropertyCache().keySet());
+    assertEquals(Sets.newHashSet(CACHED), c.getPropertyCache().keySet());
+
+    a.removeProperty(NON_CACHED);
+    b.removeProperty(NON_CACHED);
+    c.removeProperty(NON_CACHED);
+    a.removeProperty(CACHED);
+    b.removeProperty(CACHED);
+    c.removeProperty(CACHED);
+
+    assertEquals(null, a.getProperty(NON_CACHED));
+    assertEquals(null, b.getProperty(NON_CACHED));
+    assertEquals(null, c.getProperty(NON_CACHED));
+    assertEquals(null, a.getProperty(CACHED));
+    assertEquals(null, b.getProperty(CACHED));
+    assertEquals(null, c.getProperty(CACHED));
+
+    assertEquals(null, a.getPropertyCache().get(NON_CACHED));
+    assertEquals(null, b.getPropertyCache().get(NON_CACHED));
+    assertEquals(null, c.getPropertyCache().get(NON_CACHED));
+    assertEquals(null, a.getPropertyCache().get(CACHED));
+    assertEquals(null, b.getPropertyCache().get(CACHED));
+    assertEquals(null, c.getPropertyCache().get(CACHED));
+
+    assertEquals(Sets.newHashSet(), a.getPropertyCache().keySet());
+    assertEquals(Sets.newHashSet(), b.getPropertyCache().keySet());
+    assertEquals(Sets.newHashSet(), c.getPropertyCache().keySet());
+
+    graph.shutdown();
+  }
+
+  private static Graph open(AccumuloGraphConfiguration cfg) {
+    return GraphFactory.open(cfg);
+  }
+
+  private static void load(Graph graph) {
+    graph.addVertex("A");
+    graph.addVertex("B").setProperty(NON_CACHED, true);
+    graph.addVertex("C").setProperty(CACHED, true);
+  }
+}