Skip to content

Commit c9acaa2

Browse files
rashtaorashtao
authored
[DE-1068] fixed id prefix check in compex graph type (#17)
1 parent 02a810a commit c9acaa2

15 files changed

+532
-396
lines changed

‎src/main/java/com/arangodb/tinkerpop/gremlin/persistence/ElementIdFactory.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ protected ElementIdFactory(ArangoDBGraphConfig config) {
5353

5454
protected abstract String inferCollection(final String collection, final String label, final String defaultCollection);
5555

56-
protected abstract void validateId(String id);
56+
protected abstract void validateId(String id, String label);
5757

5858
protected abstract ElementId doCreate(String prefix, String collection, String key);
5959

@@ -132,7 +132,7 @@ private ElementId createId(String label, Object nullableId, String defaultCollec
132132
}
133133

134134
String id = (String) nullableId;
135-
validateId(id);
135+
validateId(id, label);
136136
return of(config.graphName, inferCollection(extractCollection(id), label, defaultCollection), extractKey(id));
137137
}
138138

‎src/main/java/com/arangodb/tinkerpop/gremlin/persistence/complex/ComplexElementIdFactory.java‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,21 @@ protected String inferCollection(final String collection, final String label, fi
5454
}
5555

5656
@Override
57-
protected void validateId(String id) {
58-
if (id.replaceFirst("^" + config.prefix, "").contains("_")) {
57+
protected void validateId(String id, String label) {
58+
if (id.contains("_")) {
5959
throw new IllegalArgumentException(String.format("id (%s) contains invalid character '_'", id));
6060
}
61+
int idx = id.indexOf('/');
62+
if (idx <= 0) {
63+
String l = label != null ? label : "<label>";
64+
throw new IllegalArgumentException(String.format("id (%s) must start with label prefix (%s/)", id, l));
65+
}
66+
if (idx >= id.length() - 1) {
67+
throw new IllegalArgumentException(String.format("id (%s) must have format (<label>/<key>)", id));
68+
}
69+
if (label != null && !id.substring(0, idx).equals(label)) {
70+
throw new IllegalArgumentException(String.format("id (%s) must start with label prefix (%s/)", id, label));
71+
}
6172
}
6273

6374
@Override

‎src/main/java/com/arangodb/tinkerpop/gremlin/persistence/simple/SimpleElementIdFactory.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ protected String inferCollection(final String collection, final String label, fi
5050
}
5151

5252
@Override
53-
protected void validateId(String id) {
53+
protected void validateId(String id, String label) {
5454
if (id.contains("_")) {
5555
throw new IllegalArgumentException(String.format("id (%s) contains invalid character '_'", id));
5656
}

‎src/main/java/com/arangodb/tinkerpop/gremlin/structure/ArangoDBEdge.java‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,7 @@
2727

2828
public class ArangoDBEdge extends ArangoDBSimpleElement<EdgeData> implements Edge, ArangoDBPersistentElement {
2929

30-
static ArangoDBEdge of(String label, ElementId id, ElementId outVertexId, ElementId inVertexId, ArangoDBGraph graph) {
31-
String inferredLabel = label != null ? label : Optional.ofNullable(id.getLabel()).orElse(Edge.DEFAULT_LABEL);
32-
return new ArangoDBEdge(graph, new EdgeData(inferredLabel, id, outVertexId, inVertexId));
33-
}
34-
35-
public ArangoDBEdge(ArangoDBGraph graph, EdgeData data) {
30+
ArangoDBEdge(ArangoDBGraph graph, EdgeData data) {
3631
super(graph, data);
3732
}
3833

‎src/main/java/com/arangodb/tinkerpop/gremlin/structure/ArangoDBGraph.java‎

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@
2121
import com.arangodb.ArangoDatabase;
2222
import com.arangodb.model.AqlQueryOptions;
2323
import com.arangodb.tinkerpop.gremlin.PackageVersion;
24-
import com.arangodb.tinkerpop.gremlin.persistence.ElementId;
25-
import com.arangodb.tinkerpop.gremlin.persistence.ElementIdFactory;
26-
import com.arangodb.tinkerpop.gremlin.persistence.VariablesData;
24+
import com.arangodb.tinkerpop.gremlin.persistence.*;
2725
import com.arangodb.tinkerpop.gremlin.process.traversal.step.sideEffect.AQLStartStep;
2826
import com.arangodb.tinkerpop.gremlin.utils.ArangoDBUtil;
2927
import org.apache.commons.configuration2.Configuration;
@@ -109,16 +107,7 @@ public ArangoDBGraphConfig.GraphType type() {
109107

110108
@Override
111109
public Vertex addVertex(Object... keyValues) {
112-
ElementHelper.legalPropertyKeyValueArray(keyValues);
113-
String label = ElementHelper.getLabelValue(keyValues).orElse(null);
114-
Object id = ElementHelper.getIdValue(keyValues).orElse(null);
115-
ElementId elementId = idFactory.createVertexId(label, id);
116-
for (int i = 0; i < keyValues.length; i = i + 2) {
117-
if (keyValues[i] instanceof String) {
118-
ArangoDBUtil.validateProperty((String) keyValues[i], keyValues[i + 1]);
119-
}
120-
}
121-
ArangoDBVertex vertex = ArangoDBVertex.of(label, elementId, this);
110+
ArangoDBVertex vertex = createVertex(keyValues);
122111
if (!config.vertices.contains(vertex.collection())) {
123112
throw new IllegalArgumentException(String.format("Vertex collection (%s) not in graph (%s).", vertex.collection(), name()));
124113
}
@@ -172,10 +161,6 @@ public ArangoDBGraphClient getClient() {
172161
return client;
173162
}
174163

175-
ElementIdFactory getIdFactory() {
176-
return idFactory;
177-
}
178-
179164
/**
180165
* Create the {@link ElementId} for a persistent element.
181166
* The returned elementId can be used to reference the related ArangoDB document in AQL queries.
@@ -294,4 +279,37 @@ public ArangoDatabase getArangoDatabase() {
294279
public ArangoGraph getArangoGraph() {
295280
return client.getArangoGraph();
296281
}
282+
283+
public ArangoDBVertex createVertex(Object... keyValues) {
284+
ElementHelper.legalPropertyKeyValueArray(keyValues);
285+
String label = ElementHelper.getLabelValue(keyValues).orElse(null);
286+
Object id = ElementHelper.getIdValue(keyValues).orElse(null);
287+
ElementId elementId = idFactory.createVertexId(label, id);
288+
for (int i = 0; i < keyValues.length; i = i + 2) {
289+
if (keyValues[i] instanceof String) {
290+
ArangoDBUtil.validateProperty((String) keyValues[i], keyValues[i + 1]);
291+
}
292+
}
293+
String inferredLabel = label != null ? label : Optional.ofNullable(elementId.getLabel()).orElse(Vertex.DEFAULT_LABEL);
294+
return createVertex(new VertexData(inferredLabel, elementId));
295+
}
296+
297+
public ArangoDBVertex createVertex(VertexData data) {
298+
return new ArangoDBVertex(this, data);
299+
}
300+
301+
public ArangoDBEdge createEdge(String label, Vertex outVertex, Vertex inVertex, Object... keyValues) {
302+
ElementHelper.legalPropertyKeyValueArray(keyValues);
303+
ElementHelper.validateLabel(label);
304+
Object id = ElementHelper.getIdValue(keyValues).orElse(null);
305+
ElementId elementId = idFactory.createEdgeId(label, id);
306+
ElementId outVertexId = idFactory.parseVertexId(outVertex.id());
307+
ElementId inVertexId = idFactory.parseVertexId(inVertex.id());
308+
return createEdge(new EdgeData(label, elementId, outVertexId, inVertexId));
309+
}
310+
311+
public ArangoDBEdge createEdge(EdgeData data) {
312+
return new ArangoDBEdge(this, data);
313+
}
314+
297315
}

‎src/main/java/com/arangodb/tinkerpop/gremlin/structure/ArangoDBVertex.java‎

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package com.arangodb.tinkerpop.gremlin.structure;
1818

19-
import com.arangodb.tinkerpop.gremlin.persistence.ElementId;
2019
import com.arangodb.tinkerpop.gremlin.persistence.VertexData;
2120
import com.arangodb.tinkerpop.gremlin.persistence.VertexPropertyData;
2221
import org.apache.tinkerpop.gremlin.structure.*;
@@ -31,12 +30,7 @@
3130

3231
public class ArangoDBVertex extends ArangoDBElement<VertexPropertyData, VertexData> implements Vertex, ArangoDBPersistentElement {
3332

34-
static ArangoDBVertex of(String label, ElementId id, ArangoDBGraph graph) {
35-
String inferredLabel = label != null ? label : Optional.ofNullable(id.getLabel()).orElse(Vertex.DEFAULT_LABEL);
36-
return new ArangoDBVertex(graph, new VertexData(inferredLabel, id));
37-
}
38-
39-
public ArangoDBVertex(ArangoDBGraph graph, VertexData data) {
33+
ArangoDBVertex(ArangoDBGraph graph, VertexData data) {
4034
super(graph, data);
4135
}
4236

@@ -74,14 +68,7 @@ public <V> VertexProperty<V> property(
7468
public Edge addEdge(String label, Vertex vertex, Object... keyValues) {
7569
if (null == vertex) throw Graph.Exceptions.argumentCanNotBeNull("vertex");
7670
if (removed() || ((ArangoDBVertex) vertex).removed()) throw elementAlreadyRemoved(id());
77-
78-
ElementHelper.legalPropertyKeyValueArray(keyValues);
79-
ElementHelper.validateLabel(label);
80-
Object id = ElementHelper.getIdValue(keyValues).orElse(null);
81-
ElementId elementId = graph.getIdFactory().createEdgeId(label, id);
82-
ElementId outVertexId = graph.getIdFactory().parseVertexId(id());
83-
ElementId inVertexId = graph.getIdFactory().parseVertexId(vertex.id());
84-
ArangoDBEdge edge = ArangoDBEdge.of(label, elementId, outVertexId, inVertexId, graph);
71+
ArangoDBEdge edge = graph.createEdge(label, this, vertex, keyValues);
8572
if (!graph.edgeCollections().contains(edge.collection())) {
8673
throw new IllegalArgumentException(String.format("Edge collection (%s) not in graph (%s).", edge.collection(), graph.name()));
8774
}

‎src/main/java/com/arangodb/tinkerpop/gremlin/utils/AqlDeserializer.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@
1818

1919
import com.arangodb.tinkerpop.gremlin.persistence.EdgeData;
2020
import com.arangodb.tinkerpop.gremlin.persistence.VertexData;
21-
import com.arangodb.tinkerpop.gremlin.structure.ArangoDBEdge;
2221
import com.arangodb.tinkerpop.gremlin.structure.ArangoDBGraph;
23-
import com.arangodb.tinkerpop.gremlin.structure.ArangoDBVertex;
2422
import com.fasterxml.jackson.databind.JsonNode;
2523
import com.fasterxml.jackson.databind.ObjectMapper;
2624
import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
@@ -52,10 +50,10 @@ private Object doDeserialize(JsonNode node) throws IOException {
5250
return null;
5351
} else if (isEdge(node)) {
5452
EdgeData data = mapper.readerFor(EdgeData.class).readValue(node);
55-
return new ArangoDBEdge(graph, data);
53+
return graph.createEdge(data);
5654
} else if (isVertex(node)) {
5755
VertexData data = mapper.readerFor(VertexData.class).readValue(node);
58-
return new ArangoDBVertex(graph, data);
56+
return graph.createVertex(data);
5957
} else if (node.isArray()) {
6058
ArrayList<Object> out = new ArrayList<>();
6159
for (JsonNode e : IteratorUtils.list(node.elements())) {

‎src/test/java/com/arangodb/tinkerpop/gremlin/arangodb/complex/ComplexElementIdTest.java‎

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,6 @@ public class ComplexElementIdTest extends AbstractGremlinTest {
3131
public void id() {
3232
assertThat(graph.addVertex(T.id, "foo/a").id()).isEqualTo("foo/a");
3333
assertThat(graph.addVertex(T.id, "foo/b", T.label, "foo").id()).isEqualTo("foo/b");
34-
assertThat(graph.addVertex(T.id, "c", T.label, "foo").id()).isEqualTo("foo/c");
35-
assertThat(graph.addVertex(T.id, "d").id()).isEqualTo(Vertex.DEFAULT_LABEL + "/d");
3634
assertThat(graph.addVertex(T.label, "foo").id())
3735
.isInstanceOf(String.class)
3836
.asString()
@@ -42,9 +40,26 @@ public void id() {
4240
.asString()
4341
.startsWith(Vertex.DEFAULT_LABEL + "/");
4442

43+
assertThat(catchThrowable(() -> graph.addVertex(T.id, "/c")))
44+
.isInstanceOf(IllegalArgumentException.class)
45+
.hasMessageContaining("must start with label prefix")
46+
.hasMessageContaining("<label>/");
47+
assertThat(catchThrowable(() -> graph.addVertex(T.id, "c/")))
48+
.isInstanceOf(IllegalArgumentException.class)
49+
.hasMessageContaining("must have format")
50+
.hasMessageContaining("<label>/<key>");
51+
assertThat(catchThrowable(() -> graph.addVertex(T.id, "c", T.label, "foo")))
52+
.isInstanceOf(IllegalArgumentException.class)
53+
.hasMessageContaining("must start with label prefix")
54+
.hasMessageContaining("foo/");
55+
assertThat(catchThrowable(() -> graph.addVertex(T.id, "d")))
56+
.isInstanceOf(IllegalArgumentException.class)
57+
.hasMessageContaining("must start with label prefix")
58+
.hasMessageContaining("<label>/");
4559
assertThat(catchThrowable(() -> graph.addVertex(T.id, "foo/bar", T.label, "baz")))
4660
.isInstanceOf(IllegalArgumentException.class)
47-
.hasMessageContaining("Mismatching label");
61+
.hasMessageContaining("must start with label prefix")
62+
.hasMessageContaining("baz/");
4863
assertThat(catchThrowable(() -> graph.addVertex(T.id, "foo/bar/baz")))
4964
.isInstanceOf(IllegalArgumentException.class)
5065
.hasMessageContaining("bar/baz")
@@ -71,11 +86,17 @@ public void id() {
7186
public void label() {
7287
assertThat(graph.addVertex(T.id, "foo/a").label()).isEqualTo("foo");
7388
assertThat(graph.addVertex(T.id, "foo/b", T.label, "foo").label()).isEqualTo("foo");
74-
assertThat(graph.addVertex(T.id, "c", T.label, "foo").label()).isEqualTo("foo");
75-
assertThat(graph.addVertex(T.id, "d").label()).isEqualTo(Vertex.DEFAULT_LABEL);
7689
assertThat(graph.addVertex(T.label, "foo").label()).isEqualTo("foo");
7790
assertThat(graph.addVertex().label()).isEqualTo(Vertex.DEFAULT_LABEL);
7891

92+
assertThat(catchThrowable(() -> graph.addVertex(T.id, "c", T.label, "foo")))
93+
.isInstanceOf(IllegalArgumentException.class)
94+
.hasMessageContaining("must start with label prefix")
95+
.hasMessageContaining("foo/");
96+
assertThat(catchThrowable(() -> graph.addVertex(T.id, "d")))
97+
.isInstanceOf(IllegalArgumentException.class)
98+
.hasMessageContaining("must start with label prefix")
99+
.hasMessageContaining("<label>/");
79100
assertThat(catchThrowable(() -> graph.addVertex(T.label, "foo/bar")))
80101
.isInstanceOf(IllegalArgumentException.class)
81102
.hasMessageContaining("foo/bar")
@@ -89,7 +110,13 @@ public void label() {
89110
@Test
90111
public void prefix() {
91112
String prefix = ((ArangoDBGraph) graph).name() + "_";
92-
assertThat(graph.addVertex(T.id, prefix + "foo/a").id()).isEqualTo("foo/a");
93-
assertThat(graph.addVertex(T.id, prefix + "foo/b", T.label, "foo").id()).isEqualTo("foo/b");
113+
assertThat(catchThrowable(() -> graph.addVertex(T.id, prefix + "foo/a")))
114+
.isInstanceOf(IllegalArgumentException.class)
115+
.hasMessageContaining(prefix + "foo/a")
116+
.hasMessageContaining("invalid character '_'");
117+
assertThat(catchThrowable(() -> graph.addVertex(T.id, prefix + "foo/b", T.label, "foo")))
118+
.isInstanceOf(IllegalArgumentException.class)
119+
.hasMessageContaining(prefix + "foo/b")
120+
.hasMessageContaining("invalid character '_'");
94121
}
95122
}

‎src/test/java/com/arangodb/tinkerpop/gremlin/arangodb/complex/ComplexPersistenceTest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ private String graphName() {
4343
@Test
4444
@SuppressWarnings("unchecked")
4545
public void vertices() {
46-
Vertex v = graph.addVertex(T.id, "foo");
46+
Vertex v = graph.addVertex(T.id, Vertex.DEFAULT_LABEL + "/foo");
4747
v
4848
.property("key", "value")
4949
.property("meta", "metaValue");
@@ -71,9 +71,9 @@ public void vertices() {
7171
@Test
7272
@SuppressWarnings("unchecked")
7373
public void edges() {
74-
Vertex a = graph.addVertex(T.id, "a");
75-
Vertex b = graph.addVertex(T.id, "b");
76-
Edge e = a.addEdge(Edge.DEFAULT_LABEL, b, T.id, "e", "key", "value");
74+
Vertex a = graph.addVertex(T.id, Vertex.DEFAULT_LABEL + "/a");
75+
Vertex b = graph.addVertex(T.id, Vertex.DEFAULT_LABEL + "/b");
76+
Edge e = a.addEdge(Edge.DEFAULT_LABEL, b, T.id, Edge.DEFAULT_LABEL + "/e", "key", "value");
7777

7878
String vertexColName = graphName() + "_" + Vertex.DEFAULT_LABEL;
7979
String edgeColName = graphName() + "_" + Edge.DEFAULT_LABEL;

0 commit comments

Comments
 (0)