Permalink
Browse files

stricter Element property key/value testing. Only change, NULL no lon…

…ger supported in GraphSON.
  • Loading branch information...
1 parent f0db8e4 commit fa717e2b7270d1a7cf8dcd5b566ead158cc808cb @okram okram committed Mar 10, 2013
View
@@ -15,6 +15,7 @@ h3. Version 2.3.0 (NOT OFFICIALLY RELEASED YET)
</dependency>
```
+* Stricter testing of property keys/value to ensure legal Blueprints values (no id, label, empty string, null)
* @Element.getProperty()@ API signature changed to support automatic typing to declared variable type
* Moved @PropertyGraphSail@, which provides a dynamic RDF view of any Blueprints graph, from Tinkubator into @blueprints-graph-sail@
* Bumped major versions of Ripple and SesameTools
@@ -5,8 +5,6 @@
import com.tinkerpop.blueprints.Element;
import com.tinkerpop.blueprints.Vertex;
import com.tinkerpop.blueprints.util.ElementHelper;
-import com.tinkerpop.blueprints.util.ExceptionFactory;
-import com.tinkerpop.blueprints.util.StringFactory;
import java.io.Serializable;
import java.util.HashMap;
@@ -37,13 +35,7 @@ protected TinkerElement(final String id, final TinkerGraph graph) {
}
public void setProperty(final String key, final Object value) {
- if (key.equals(StringFactory.ID))
- throw ExceptionFactory.propertyKeyIdIsReserved();
- if (key.equals(StringFactory.LABEL) && this instanceof Edge)
- throw ExceptionFactory.propertyKeyLabelIsReservedForEdges();
- if (key.equals(StringFactory.EMPTY_STRING))
- throw ExceptionFactory.elementKeyCanNotBeEmpty();
-
+ ElementHelper.validateProperty(this, key, value);
Object oldValue = this.properties.put(key, value);
if (this instanceof TinkerVertex)
this.graph.vertexKeyIndex.autoUpdate(key, value, oldValue, (TinkerVertex) this);
@@ -1,5 +1,6 @@
package com.tinkerpop.blueprints.util;
+import com.tinkerpop.blueprints.Edge;
import com.tinkerpop.blueprints.Element;
import java.util.ArrayList;
@@ -13,6 +14,28 @@
*/
public class ElementHelper {
+ /**
+ * Determines whether the property key/value for the specified element can be legally set.
+ * This is typically used as a pre-condition check prior to setting a property.
+ *
+ * @param element the element for the property to be set
+ * @param key the key of the property
+ * @param value the value of the property
+ * @throws IllegalArgumentException whether the triple is legal and if not, a clear reason message is provided
+ */
+ public static final void validateProperty(final Element element, final String key, final Object value) throws IllegalArgumentException {
+ if (null == value)
+ throw ExceptionFactory.propertyValueCanNotBeNull();
+ if (null == key)
+ throw ExceptionFactory.propertyKeyCanNotBeNull();
+ if (key.equals(StringFactory.ID))
+ throw ExceptionFactory.propertyKeyIdIsReserved();
+ if (element instanceof Edge && key.equals(StringFactory.LABEL))
+ throw ExceptionFactory.propertyKeyLabelIsReservedForEdges();
+ if (key.isEmpty())
+ throw ExceptionFactory.propertyKeyCanNotBeEmpty();
+ }
+
/**
* Copy the properties (key and value) from one element to another.
* The properties are preserved on the from element.
@@ -44,10 +44,18 @@ public static IllegalArgumentException propertyKeyLabelIsReservedForEdges() {
return new IllegalArgumentException("Property key is reserved for all edges: label");
}
- public static IllegalArgumentException elementKeyCanNotBeEmpty() {
+ public static IllegalArgumentException propertyKeyCanNotBeEmpty() {
return new IllegalArgumentException("Property key can not be the empty string");
}
+ public static IllegalArgumentException propertyKeyCanNotBeNull() {
+ return new IllegalArgumentException("Property key can not be null");
+ }
+
+ public static IllegalArgumentException propertyValueCanNotBeNull() {
+ return new IllegalArgumentException("Property value can not be null");
+ }
+
// IndexableGraph related exceptions
public static IllegalArgumentException indexAlreadyExists(final String indexName) {
@@ -7,7 +7,6 @@
import com.tinkerpop.blueprints.Element;
import com.tinkerpop.blueprints.Vertex;
import com.tinkerpop.blueprints.util.ElementHelper;
-import com.tinkerpop.blueprints.util.ExceptionFactory;
import com.tinkerpop.blueprints.util.StringFactory;
import java.util.HashSet;
@@ -160,15 +159,10 @@ protected DexElement(final DexGraph g, final long oid) {
*/
@Override
public void setProperty(final String key, final Object value) {
- graph.autoStartTransaction();
-
- //System.out.println(this + "!!" + key + "!!" + value);
- if (key.equals(StringFactory.ID))
- throw ExceptionFactory.propertyKeyIdIsReserved();
+ ElementHelper.validateProperty(this, key, value);
if (key.equals(StringFactory.LABEL))
- throw new IllegalArgumentException("Property key is reserved for all nodes and edges: " + StringFactory.LABEL);
- if (key.equals(StringFactory.EMPTY_STRING))
- throw ExceptionFactory.elementKeyCanNotBeEmpty();
+ throw new IllegalArgumentException("Property key is reserved for all vertices and edges: " + StringFactory.LABEL);
+ graph.autoStartTransaction();
int attr = graph.getRawGraph().findAttribute(getObjectType(), key);
com.sparsity.dex.gdb.DataType datatype = null;
@@ -37,13 +37,7 @@ public Neo4jElement(final Neo4jGraph graph) {
}
public void setProperty(final String key, final Object value) {
- if (key.isEmpty())
- throw ExceptionFactory.elementKeyCanNotBeEmpty();
- if (key.equals(StringFactory.ID))
- throw ExceptionFactory.propertyKeyIdIsReserved();
- if (key.equals(StringFactory.LABEL) && this instanceof Edge)
- throw ExceptionFactory.propertyKeyLabelIsReservedForEdges();
-
+ ElementHelper.validateProperty(this, key, value);
this.graph.autoStartTransaction();
// attempts to take a collection and convert it to an array so that Neo4j can consume it
this.rawElement.setProperty(key, tryConvertCollectionToArray(value));
@@ -30,7 +30,7 @@ public Neo4jBatchEdge(final Neo4jBatchGraph graph, final Long id, final String l
public void setProperty(final String key, final Object value) {
if (key.isEmpty())
- throw ExceptionFactory.elementKeyCanNotBeEmpty();
+ throw ExceptionFactory.propertyKeyCanNotBeEmpty();
if (key.equals(StringFactory.ID))
throw ExceptionFactory.propertyKeyIdIsReserved();
if (key.equals(StringFactory.LABEL))
@@ -28,7 +28,7 @@ public Neo4jBatchVertex(final Neo4jBatchGraph graph, final Long id) {
public void setProperty(final String key, final Object value) {
if (key.isEmpty())
- throw ExceptionFactory.elementKeyCanNotBeEmpty();
+ throw ExceptionFactory.propertyKeyCanNotBeEmpty();
if (key.equals(StringFactory.ID))
throw ExceptionFactory.propertyKeyIdIsReserved();
@@ -32,13 +32,7 @@ protected OrientElement(final OrientBaseGraph rawGraph, final ODocument rawEleme
}
public void setProperty(final String key, final Object value) {
- if (key.equals(StringFactory.ID))
- throw ExceptionFactory.propertyKeyIdIsReserved();
- if (key.equals(StringFactory.LABEL) && this instanceof Edge)
- throw ExceptionFactory.propertyKeyLabelIsReservedForEdges();
- if (key.equals(StringFactory.EMPTY_STRING))
- throw ExceptionFactory.elementKeyCanNotBeEmpty();
-
+ ElementHelper.validateProperty(this, key, value);
this.graph.autoStartTransaction();
this.rawElement.field(key, value);
this.graph.getRawGraph().save(rawElement);
@@ -78,13 +78,7 @@ public void remove() {
}
public void setProperty(final String key, final Object value) {
- if (key.equals(StringFactory.ID))
- throw ExceptionFactory.propertyKeyIdIsReserved();
- if (key.equals(StringFactory.LABEL) && this instanceof Edge)
- throw ExceptionFactory.propertyKeyLabelIsReservedForEdges();
- if (key.equals(StringFactory.EMPTY_STRING))
- throw ExceptionFactory.elementKeyCanNotBeEmpty();
-
+ ElementHelper.validateProperty(this, key, value);
if (key.startsWith(RexsterTokens.UNDERSCORE))
throw new RuntimeException("RexsterGraph does not support property keys that start with underscore");
@@ -1,6 +1,7 @@
package com.tinkerpop.blueprints;
import com.tinkerpop.blueprints.impls.GraphTest;
+import com.tinkerpop.blueprints.util.StringFactory;
import java.util.HashSet;
import java.util.Set;
@@ -603,4 +604,36 @@ public void testEdgeCentricRemoving() {
graph.shutdown();
}
+
+ public void testSettingBadVertexProperties() {
+ final Graph graph = graphTest.generateGraph();
+ if (graph.getFeatures().supportsVertexProperties) {
+ Edge edge = graph.addEdge(null, graph.addVertex(null), graph.addVertex(null), "knows");
+ try {
+ edge.setProperty(null, -1);
+ assertFalse(true);
+ } catch (RuntimeException e) {
+ assertTrue(true);
+ }
+ try {
+ edge.setProperty("", -1);
+ assertFalse(true);
+ } catch (RuntimeException e) {
+ assertTrue(true);
+ }
+ try {
+ edge.setProperty(StringFactory.ID, -1);
+ assertFalse(true);
+ } catch (RuntimeException e) {
+ assertTrue(true);
+ }
+ try {
+ edge.setProperty(StringFactory.LABEL, "friend");
+ assertFalse(true);
+ } catch (RuntimeException e) {
+ assertTrue(true);
+ }
+ }
+ graph.shutdown();
+ }
}
@@ -2,6 +2,7 @@
import com.tinkerpop.blueprints.impls.GraphTest;
import com.tinkerpop.blueprints.impls.sail.SailTokens;
+import com.tinkerpop.blueprints.util.StringFactory;
import java.util.HashSet;
import java.util.Set;
@@ -589,4 +590,30 @@ public void testConcurrentModificationOnProperties() {
}
graph.shutdown();
}
+
+ public void testSettingBadVertexProperties() {
+ final Graph graph = graphTest.generateGraph();
+ if (graph.getFeatures().supportsVertexProperties) {
+ Vertex v = graph.addVertex(null);
+ try {
+ v.setProperty(null,-1);
+ assertFalse(true);
+ } catch(RuntimeException e) {
+ assertTrue(true);
+ }
+ try {
+ v.setProperty("",-1);
+ assertFalse(true);
+ } catch(RuntimeException e) {
+ assertTrue(true);
+ }
+ try {
+ v.setProperty(StringFactory.ID,-1);
+ assertFalse(true);
+ } catch(RuntimeException e) {
+ assertTrue(true);
+ }
+ }
+ graph.shutdown();
+ }
}
@@ -25,7 +25,7 @@
public void inputGraphModeExtended() throws IOException {
TinkerGraph graph = new TinkerGraph();
- String json = "{ \"mode\":\"EXTENDED\", \"vertices\": [ {\"_id\":1, \"_type\":\"vertex\", \"test\": { \"type\":\"string\", \"value\":\"please work\"}, \"testlist\":{\"type\":\"list\", \"value\":[{\"type\":\"int\", \"value\":1}, {\"type\":\"int\",\"value\":2}, {\"type\":\"int\",\"value\":3}, {\"type\":\"unknown\",\"value\":null}]}, \"testmap\":{\"type\":\"map\", \"value\":{\"big\":{\"type\":\"long\", \"value\":10000000000}, \"small\":{\"type\":\"double\", \"value\":0.4954959595959}, \"nullKey\":{\"type\":\"unknown\", \"value\":null}}}}, {\"_id\":2, \"_type\":\"vertex\", \"testagain\":{\"type\":\"string\", \"value\":\"please work again\"}}], \"edges\":[{\"_id\":100, \"_type\":\"edge\", \"_outV\":1, \"_inV\":2, \"_label\":\"works\", \"teste\": {\"type\":\"string\", \"value\":\"please worke\"}, \"keyNull\":{\"type\":\"unknown\", \"value\":null}}]}";
+ String json = "{ \"mode\":\"EXTENDED\", \"vertices\": [ {\"_id\":1, \"_type\":\"vertex\", \"test\": { \"type\":\"string\", \"value\":\"please work\"}, \"testlist\":{\"type\":\"list\", \"value\":[{\"type\":\"int\", \"value\":1}, {\"type\":\"int\",\"value\":2}, {\"type\":\"int\",\"value\":3}]}, \"testmap\":{\"type\":\"map\", \"value\":{\"big\":{\"type\":\"long\", \"value\":10000000000}, \"small\":{\"type\":\"double\", \"value\":0.4954959595959}}}}, {\"_id\":2, \"_type\":\"vertex\", \"testagain\":{\"type\":\"string\", \"value\":\"please work again\"}}], \"edges\":[{\"_id\":100, \"_type\":\"edge\", \"_outV\":1, \"_inV\":2, \"_label\":\"works\", \"teste\": {\"type\":\"string\", \"value\":\"please worke\"}}]}";
byte[] bytes = json.getBytes();
InputStream inputStream = new ByteArrayInputStream(bytes);
@@ -43,10 +43,10 @@ public void inputGraphModeExtended() throws IOException {
Assert.assertNotNull(map);
Assert.assertEquals(10000000000l, Long.parseLong(map.get("big").toString()));
Assert.assertEquals(0.4954959595959, Double.parseDouble(map.get("small").toString()), 0);
- Assert.assertNull(map.get("nullKey"));
+ //Assert.assertNull(map.get("nullKey"));
List list = (List) v1.getProperty("testlist");
- Assert.assertEquals(4, list.size());
+ Assert.assertEquals(3, list.size());
boolean foundNull = false;
for (int ix = 0; ix < list.size(); ix++) {
@@ -68,15 +68,15 @@ public void inputGraphModeExtended() throws IOException {
Assert.assertEquals(v1, e.getVertex(Direction.OUT));
Assert.assertEquals(v2, e.getVertex(Direction.IN));
Assert.assertEquals("please worke", e.getProperty("teste"));
- Assert.assertNull(e.getProperty("keyNull"));
+ //Assert.assertNull(e.getProperty("keyNull"));
}
@Test
public void inputGraphModeNormal() throws IOException {
TinkerGraph graph = new TinkerGraph();
- String json = "{ \"mode\":\"NORMAL\",\"vertices\": [ {\"_id\":1, \"_type\":\"vertex\", \"test\": \"please work\", \"testlist\":[1, 2, 3, null], \"testmap\":{\"big\":10000000000, \"small\":0.4954959595959, \"nullKey\":null}}, {\"_id\":2, \"_type\":\"vertex\", \"testagain\":\"please work again\"}], \"edges\":[{\"_id\":100, \"_type\":\"edge\", \"_outV\":1, \"_inV\":2, \"_label\":\"works\", \"teste\": \"please worke\", \"keyNull\":null}]}";
+ String json = "{ \"mode\":\"NORMAL\",\"vertices\": [ {\"_id\":1, \"_type\":\"vertex\", \"test\": \"please work\", \"testlist\":[1, 2, 3, null], \"testmap\":{\"big\":10000000000, \"small\":0.4954959595959}}, {\"_id\":2, \"_type\":\"vertex\", \"testagain\":\"please work again\"}], \"edges\":[{\"_id\":100, \"_type\":\"edge\", \"_outV\":1, \"_inV\":2, \"_label\":\"works\", \"teste\": \"please worke\"}]}";
byte[] bytes = json.getBytes();
InputStream inputStream = new ByteArrayInputStream(bytes);
@@ -127,7 +127,7 @@ public void inputGraphModeNormal() throws IOException {
public void inputGraphModeCompact() throws IOException {
TinkerGraph graph = new TinkerGraph();
- String json = "{ \"mode\":\"COMPACT\",\"vertices\": [ {\"_id\":1, \"test\": \"please work\", \"testlist\":[1, 2, 3, null], \"testmap\":{\"big\":10000000000, \"small\":0.4954959595959, \"nullKey\":null}}, {\"_id\":2, \"testagain\":\"please work again\"}], \"edges\":[{\"_id\":100, \"_outV\":1, \"_inV\":2, \"_label\":\"works\", \"teste\": \"please worke\", \"keyNull\":null}]}";
+ String json = "{ \"mode\":\"COMPACT\",\"vertices\": [ {\"_id\":1, \"test\": \"please work\", \"testlist\":[1, 2, 3, null], \"testmap\":{\"big\":10000000000, \"small\":0.4954959595959}}, {\"_id\":2, \"testagain\":\"please work again\"}], \"edges\":[{\"_id\":100, \"_outV\":1, \"_inV\":2, \"_label\":\"works\", \"teste\": \"please worke\"}]}";
byte[] bytes = json.getBytes();
InputStream inputStream = new ByteArrayInputStream(bytes);
@@ -145,7 +145,7 @@ public void inputGraphModeCompact() throws IOException {
Assert.assertNotNull(map);
Assert.assertEquals(10000000000l, Long.parseLong(map.get("big").toString()));
Assert.assertEquals(0.4954959595959, Double.parseDouble(map.get("small").toString()), 0);
- Assert.assertNull(map.get("nullKey"));
+ // Assert.assertNull(map.get("nullKey"));
List list = (List) v1.getProperty("testlist");
Assert.assertEquals(4, list.size());
@@ -863,7 +863,7 @@ public void jsonFromElementVertexMapPropertiesNoKeysWithTypes() throws JSONExcep
public void jsonFromElementNullsNoKeysNoTypes() throws JSONException {
Graph g = new TinkerGraph();
Vertex v = g.addVertex(1);
- v.setProperty("key", null);
+ //v.setProperty("key", null);
Map<String, Object> map = new HashMap<String, Object>();
map.put("innerkey", null);
@@ -904,7 +904,7 @@ public void jsonFromElementNullsNoKeysNoTypes() throws JSONException {
public void jsonFromElementNullsNoKeysWithTypes() throws JSONException {
Graph g = new TinkerGraph();
Vertex v = g.addVertex(1);
- v.setProperty("key", null);
+ // v.setProperty("key", null);
Map<String, Object> map = new HashMap<String, Object>();
map.put("innerkey", null);
@@ -925,8 +925,8 @@ public void jsonFromElementNullsNoKeysWithTypes() throws JSONException {
Assert.assertNotNull(json);
JSONObject jsonObjectKey = json.optJSONObject("key");
- Assert.assertTrue(jsonObjectKey.isNull(GraphSONTokens.VALUE));
- Assert.assertEquals(GraphSONTokens.TYPE_UNKNOWN, jsonObjectKey.optString(GraphSONTokens.TYPE));
+// Assert.assertTrue(jsonObjectKey.isNull(GraphSONTokens.VALUE));
+// Assert.assertEquals(GraphSONTokens.TYPE_UNKNOWN, jsonObjectKey.optString(GraphSONTokens.TYPE));
JSONObject jsonMap = json.optJSONObject("keyMap").optJSONObject(GraphSONTokens.VALUE);
Assert.assertNotNull(jsonMap);

0 comments on commit fa717e2

Please sign in to comment.