Github user spmallette commented on a diff in the pull request:
https://github.com/apache/incubator-tinkerpop/pull/107#discussion_r41984504
--- Diff:
gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/computer/bulkloading/BulkLoaderVertexProgramTest.java
---
@@ -60,23 +78,43 @@ public void shouldUseIncrementalBulkLoaderByDefault()
throws Exception {
@Test
@LoadGraphWith(MODERN)
public void shouldStoreOriginalIds() throws Exception {
- final BulkLoader loader =
getBulkLoader(BulkLoaderVertexProgram.build().userSuppliedIds(false).create(graph));
+ final BulkLoaderVertexProgram blvp =
BulkLoaderVertexProgram.build()
+ .userSuppliedIds(false)
+ .writeGraph(getWriteGraphConfiguration()).create(graph);
+ final BulkLoader loader = getBulkLoader(blvp);
assertFalse(loader.useUserSuppliedIds());
- final Graph target = getTargetGraph();
- graph.vertices().forEachRemaining(v -> loader.getOrCreateVertex(v,
target, target.traversal()));
+ graph.compute().program(blvp).submit().get();
+ final Graph target = getWriteGraph();
+ assertEquals(IteratorUtils.count(graph.vertices()),
IteratorUtils.count(target.vertices()));
+ assertEquals(IteratorUtils.count(graph.edges()),
IteratorUtils.count(target.edges()));
target.vertices().forEachRemaining(v ->
assertTrue(v.property(loader.getVertexIdProperty()).isPresent()));
}
@Test
@LoadGraphWith(MODERN)
public void shouldNotStoreOriginalIds() throws Exception {
- final BulkLoader loader =
getBulkLoader(BulkLoaderVertexProgram.build().userSuppliedIds(true).create(graph));
+ final BulkLoaderVertexProgram blvp =
BulkLoaderVertexProgram.build()
+ .userSuppliedIds(true)
+ .writeGraph(getWriteGraphConfiguration()).create(graph);
+ final BulkLoader loader = getBulkLoader(blvp);
assertTrue(loader.useUserSuppliedIds());
- final Graph target = getTargetGraph();
- graph.vertices().forEachRemaining(v -> loader.getOrCreateVertex(v,
target, target.traversal()));
+ graph.compute().program(blvp).submit().get();
+ final Graph target = getWriteGraph();
+ assertEquals(IteratorUtils.count(graph.vertices()),
IteratorUtils.count(target.vertices()));
+ assertEquals(IteratorUtils.count(graph.edges()),
IteratorUtils.count(target.edges()));
target.vertices().forEachRemaining(v ->
assertFalse(v.property(loader.getVertexIdProperty()).isPresent()));
}
- // TODO: once Neo4j supports concurrent connections, write a real
integration test that leverages BLVP
- // TODO: also, once Neo4j can be used, remove the tinkergraph-gremlin
dependency from hadoop-gremlin and clean up the existing tests
+ @Test
+ @LoadGraphWith(MODERN)
+ public void shouldOverwriteExistingElements() throws Exception {
+ final BulkLoaderVertexProgram blvp =
BulkLoaderVertexProgram.build()
+ .userSuppliedIds(true)
--- End diff --
@dkuppitz can you explain why this test passes when Neo4j is the graph you
are writing to? Your documentation says:
> |`userSuppliedIds(boolean)` |Whether to use the id's from the source
graph as id's in the target graph. If set to `true`, `vertexIdProperty` will be
ignored. Note, that the target graph must support user supplied identifiers. |
`false`
but Neo4j doesn't support user supplied ids. Seems like you should have a:
```java
@FeatureRequirement(featureClass = Graph.Features.EdgeFeatures.class,
feature = Graph.Features.EdgeFeatures.FEATURE_USER_SUPPLIED_IDS)
```
Am I missing something obvious here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
