[BUG] Tinkerpop DefaultTinkerpopGraphDatabaseManager.insert(CommunicationEntity) can't work with detachedVertex

[JulienDropsy]

Which JNoSQL project the issue refers to?

JNoSQL Databases

Bug description

I try to use tinkerpop jnosql with AWS Neptune Graph but i am not able to insert a new vertex with the help of tinkerpopTemplate.insert(T entity).

Error is

java.lang.IllegalStateException: Property addition is not supported
at org.apache.tinkerpop.gremlin.structure.Element$Exceptions.propertyAdditionNotSupported(Element.java:134)
at org.apache.tinkerpop.gremlin.structure.util.detached.DetachedVertex.property(DetachedVertex.java:104)
at org.eclipse.jnosql.databases.tinkerpop.communication.DefaultTinkerpopGraphDatabaseManager.lambda$insert$0(DefaultTinkerpopGraphDatabaseManager.java:77)

When I look at the code, I can see this

@Override
    public CommunicationEntity insert(CommunicationEntity entity) {

        Objects.requireNonNull(entity, "entity is required");
        Vertex vertex = graph.addVertex(entity.name());
        entity.elements().forEach(e -> vertex.property(e.name(), ValueUtil.convert(e.value())));
        entity.add(ID_PROPERTY, vertex.id());
        vertex.property(ID_PROPERTY, vertex.id());
        GraphTransactionUtil.transaction(graph);
        return entity;
    }

We can see here that there is a first call to add a vertex :

        Vertex vertex = graph.addVertex(entity.name());

This call creates a vertex but the call give me back a org.apache.tinkerpop.gremlin.structure.util.detached.DetachedVertex because I work with a Neptune Graph with a remote isntanciation :

this.cluster = 
Cluster.build().addContactPoint(endpoint).port(port).enableSsl(ssl).connectionSetupTimeoutMillis(connectTimeout)
                .keepAliveInterval(keepAliveInterval).create();
cluster.connect();
this.g = AnonymousTraversalSource.traversal().withRemote(DriverRemoteConnection.using(cluster));

At this point, I can't use vertex.property() anymore since object is detached (DetachedVertex)

@Override
    public CommunicationEntity insert(CommunicationEntity entity) {

       ...
       ...
        entity.elements().forEach(e -> vertex.property(e.name(), ValueUtil.convert(e.value()))); => ERROR
        ...
        ...
    }

Is there a reason or something I misunderstand to not have "bundle" the call in a single traversal ?

The methode call is

/**
     * Add a {@link Vertex} to the graph with provided vertex label.
     *
     * @param label the label of the vertex
     * @return The newly created labeled vertex
     */
    default Vertex addVertex(final String label) {
        return this.addVertex(T.label, label);
    }

And could have been

/**
     * Add a {@link Vertex} to the graph with provided vertex label and properties
     *
     * @param 
     *      label the label of the vertex
     *      properties the properties for the vertex
     * @return The newly created labeled vertex
     */
    default Vertex addVertex(final String label, final Object... keyValues) {
        return this.addVertex(T.label, label, keyValues);
    }

This way, the call could be performed in a "single" traversal into the Graph implementation from a remote graph connection and return a detached entity.

Not sure if it is a bug, a mandatory design choice that I dont understand or something else.

Thanks for support.

Note : I already opened an issue not related here #572 wich is closed but it is also with tinkerpop, Neptune and Quarkus and then could help for debugging as a Quarkus Test :-)

JNoSQL Version

JNoSQL version 1.1.7

Steps To Reproduce

1. Configure maven projet with jsnosql tinkerpop and Quarkus
2. Create custom classes for instanciate neptune graph
3. Run the test to insert a vertex

Expected Results

The vertex is inserted with all properties

Code example, screenshot, or link to a repository

1. I use this pom.xml

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>
    <groupId>be.demo.quarkus</groupId>
    <artifactId>test-jnosql-quarkus</artifactId>
    <version>1.0.0-SNAPSHOT</version>
    <properties>
        <maven.compiler.parameters>true</maven.compiler.parameters>
        <maven.compiler.target>17</maven.compiler.target>
        <maven.compiler.source>17</maven.compiler.source>
        <maven-compiler-plugin.version>3.13.0</maven-compiler-plugin.version>
        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
        <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
        <!-- QUARKUS -->
        <quarkus.platform.version>3.17.5</quarkus.platform.version>
        <!-- UTILS -->
        <gremlin-driver.version>3.7.3</gremlin-driver.version>
        <!-- PLUGINS -->
        <failsafe-plugin.version>3.5.2</failsafe-plugin.version>
        <jandex-maven-plugin.version>3.0.5</jandex-maven-plugin.version>
        <jib-maven-plugin.version>3.3.1</jib-maven-plugin.version>
        <surefire-plugin.version>3.5.2</surefire-plugin.version>
    </properties>

    <dependencyManagement>
        <dependencies>
            <!-- QUARKUS -->
            <dependency>
                <groupId>io.quarkus.platform</groupId>
                <artifactId>quarkus-bom</artifactId>
                <version>${quarkus.platform.version}</version>
                <type>pom</type>
                <scope>import</scope>
            </dependency>

            <!-- QUARKUS ADDONS -->
            <dependency>
                <groupId>io.quarkus.platform</groupId>
                <artifactId>quarkus-amazon-services-bom</artifactId>
                <version>${quarkus.platform.version}</version>
                <type>pom</type>
                <scope>import</scope>
            </dependency>
		</dependencies>
    </dependencyManagement>
	
    <dependencies>
	
        <dependency>
            <groupId>io.quarkus</groupId>
            <artifactId>quarkus-amazon-lambda-rest</artifactId>
        </dependency>

        <dependency>
            <groupId>io.quarkus</groupId>
            <artifactId>quarkus-rest-jackson</artifactId>
        </dependency>

        <dependency>
            <groupId>io.quarkus</groupId>
            <artifactId>quarkus-smallrye-openapi</artifactId>
        </dependency>

        <dependency>
            <groupId>io.quarkus</groupId>
            <artifactId>quarkus-smallrye-jwt</artifactId>
        </dependency>

        <dependency>
            <groupId>io.quarkus</groupId>
            <artifactId>quarkus-oidc</artifactId>
        </dependency>

        <dependency>
            <groupId>io.quarkus</groupId>
            <artifactId>quarkus-hibernate-validator</artifactId>
        </dependency>

	<!-- GRAPH -->
	<dependency>
		<groupId>org.apache.tinkerpop</groupId>
		<artifactId>gremlin-driver</artifactId>
		<version>${gremlin-driver.version}</version>
	</dependency>
	<dependency>
		<groupId>org.apache.tinkerpop</groupId>
		<artifactId>gremlin-core</artifactId>
		<version>${gremlin-driver.version}</version>
	</dependency>
	<dependency>
		<groupId>org.apache.tinkerpop</groupId>
		<artifactId>gremlin-groovy</artifactId>
		<version>${gremlin-driver.version}</version>
	</dependency>
	<dependency>
		<groupId>org.eclipse.jnosql.mapping</groupId>
		<artifactId>jnosql-graph-connections</artifactId>
		<version>1.1.7</version>
	</dependency>
	<dependency>
		<groupId>org.eclipse.jnosql.databases</groupId>
		<artifactId>jnosql-tinkerpop</artifactId>
		<version>1.1.7</version>
	</dependency>
		
		<!-- TESTS -->
        <dependency>
            <groupId>io.quarkus</groupId>
            <artifactId>quarkus-junit5</artifactId>
            <scope>test</scope>
        </dependency>
        <dependency>
            <groupId>io.quarkus</groupId>
            <artifactId>quarkus-junit5-mockito</artifactId>
            <scope>test</scope>
        </dependency>
        <dependency>
            <groupId>org.mockito</groupId>
            <artifactId>mockito-core</artifactId>
            <version>${mockito.version}</version>
            <scope>test</scope>
        </dependency>
		
    </dependencies>

    <build>
        <pluginManagement>
            <plugins>
                <plugin>
                    <!-- The classes need to be indexed to be used/injected in other module -->
                    <groupId>io.smallrye</groupId>
                    <artifactId>jandex-maven-plugin</artifactId>
                    <version>${jandex-maven-plugin.version}</version>
                    <executions>
                        <execution>
                            <id>make-index</id>
                            <goals>
                                <goal>jandex</goal>
                            </goals>
                        </execution>
                    </executions>
                </plugin>
                <plugin>
                    <groupId>io.quarkus</groupId>
                    <artifactId>quarkus-maven-plugin</artifactId>
                    <version>${quarkus.platform.version}</version>
                    <executions>
                        <execution>
                            <goals>
                                <goal>build</goal>
                            </goals>
                        </execution>
                    </executions>
                </plugin>
                <plugin>
                    <groupId>com.google.cloud.tools</groupId>
                    <artifactId>jib-maven-plugin</artifactId>
                    <version>${jib-maven-plugin.version}</version>
                </plugin>
                <plugin>
                    <artifactId>maven-surefire-plugin</artifactId>
                    <version>${surefire-plugin.version}</version>
                    <configuration>
                        <systemPropertyVariables>
                            <java.util.logging.manager>org.jboss.logmanager.LogManager</java.util.logging.manager>
                            <maven.home>${maven.home}</maven.home>
                        </systemPropertyVariables>
                    </configuration>
                </plugin>
                <plugin>
                    <artifactId>maven-failsafe-plugin</artifactId>
                    <version>${failsafe-plugin.version}</version>
                    <executions>
                        <execution>
                            <goals>
                                <goal>integration-test</goal>
                                <goal>verify</goal>
                            </goals>
                            <configuration>
                                <argLine>
                                    --initialize-at-run-time=org.apache.http.impl.auth.NTLMEngineImpl</argLine>
                                <systemPropertyVariables>
                                    <native.image.path>
                                        ${project.build.directory}/${project.build.finalName}-runner
                                    </native.image.path>
                                    <java.util.logging.manager>org.jboss.logmanager.LogManager
                                    </java.util.logging.manager>
                                    <maven.home>${maven.home}</maven.home>
                                </systemPropertyVariables>
                            </configuration>
                        </execution>
                    </executions>
                </plugin>
            </plugins>
        </pluginManagement>
        <plugins>
            <plugin>
                <artifactId>maven-compiler-plugin</artifactId>
                <version>${maven-compiler-plugin.version}</version>
                <configuration>
                    <compilerArgument>-parameters</compilerArgument>
                    <annotationProcessorPaths>
                        <path>
                            <groupId>io.quarkus</groupId>
                            <artifactId>quarkus-extension-processor</artifactId>
                            <version>${quarkus.platform.version}</version>
                        </path>
                    </annotationProcessorPaths>
                    <release>17</release>
                </configuration>
            </plugin>
        </plugins>
    </build>
</project>

2. I Implement custom class for instanciate Neptune Graph

• My custom Neptune graph implementation :

import java.util.Iterator;
import java.util.List;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;

import org.apache.commons.configuration2.BaseConfiguration;
import org.apache.commons.configuration2.Configuration;
import org.apache.tinkerpop.gremlin.driver.Cluster;
import org.apache.tinkerpop.gremlin.driver.remote.DriverRemoteConnection;
import org.apache.tinkerpop.gremlin.process.computer.GraphComputer;
import org.apache.tinkerpop.gremlin.process.traversal.AnonymousTraversalSource;
import org.apache.tinkerpop.gremlin.process.traversal.TraversalSource;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
import org.apache.tinkerpop.gremlin.structure.Edge;
import org.apache.tinkerpop.gremlin.structure.Graph;
import org.apache.tinkerpop.gremlin.structure.Transaction;
import org.apache.tinkerpop.gremlin.structure.Vertex;
import org.apache.tinkerpop.gremlin.structure.util.ElementHelper;
import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph;
import org.eclipse.microprofile.config.Config;
import org.eclipse.microprofile.config.ConfigProvider;

public class NeptuneGraph implements Graph {

    private final Cluster cluster;
    private final GraphTraversalSource g;
    private final Configuration configuration;

    private final AtomicReference<Transaction> transaction = new AtomicReference<>();

    public NeptuneGraph() {

        Config config = ConfigProvider.getConfig();

        String endpoint = config.getValue("NEPTUNE_ENDPOINT", String.class);
        int port = config.getOptionalValue("NEPTUNE_PORT", Integer.class).orElse(8182);
        boolean ssl = config.getOptionalValue("NEPTUNE_CONNECTION_TIMEOUT", Boolean.class).orElse(true);
        long connectTimeout = config.getOptionalValue("NEPTUNE_CONNECTION_TIMEOUT", Long.class).orElse(10000L);
        long keepAliveInterval = config.getOptionalValue("NEPTUNE_KEEP_ALIVE_INTERVAL", Long.class).orElse(15000L);

        this.cluster = Cluster.build().addContactPoint(endpoint).port(port).enableSsl(ssl).connectionSetupTimeoutMillis(connectTimeout)
                .keepAliveInterval(keepAliveInterval).create();

        cluster.connect();
        this.g = AnonymousTraversalSource.traversal().withRemote(DriverRemoteConnection.using(cluster));

        this.configuration = new BaseConfiguration();
        configuration.addProperty("neptune.endpoint", endpoint);
        configuration.addProperty("neptune.port", port);
        configuration.addProperty("neptune.ssl", ssl);
        configuration.addProperty("neptune.keepAliveInterval", keepAliveInterval);
    }

    @Override
    public Vertex addVertex(Object... keyValues) {

        ElementHelper.legalPropertyKeyValueArray(keyValues);
        String label = ElementHelper.getLabelValue(keyValues).orElse(Vertex.DEFAULT_LABEL);
        if (label == null) {
            throw Graph.Exceptions.argumentCanNotBeNull("label");
        }

        List<Object> keyValuesList = List.of(keyValues);

        if (keyValuesList.size() < 2) {
            throw Graph.Exceptions.argumentCanNotBeNull("keyValues must contain at least a key and a value");
        }
        if (keyValuesList.size() % 2 != 0) {
            throw Graph.Exceptions.argumentCanNotBeNull("keyValues must contain an even number of elements");
        }

        return g.addV(label).next();
    }

    @Override
    public <C extends GraphComputer> C compute(Class<C> graphComputerClass) throws IllegalArgumentException {
        throw Graph.Exceptions.graphComputerNotSupported();
    }

    @Override
    public GraphTraversalSource traversal() {
        return g;
    }

    @Override
    public Configuration configuration() {
        return configuration;
    }

    @Override
    public Variables variables() {
        return EmptyGraph.instance().variables();
    }

    @Override
    public void close() {
        if (g != null) {
            try {
                g.close();
            } catch (Exception e) {
                throw new RuntimeException(e);
            }
        }
        if (cluster != null) {
            cluster.close();
        }
    }

    @Override
    public GraphComputer compute() throws IllegalArgumentException {
        throw new UnsupportedOperationException("Neptune does not support it");
    }

    @Override
    public Transaction tx() {

        if (transaction.get() == null) {

            transaction.set(new Transaction() {

                @Override
                public <T extends TraversalSource> T begin(Class<T> traversalSourceClass) {
                    return g.tx().begin(traversalSourceClass);
                }

                @Override
                public void open() {
                    // Neptune implicitly opens a session with g.tx().open()
                    g.tx().open();
                }

                @Override
                public Transaction onClose(Consumer<Transaction> consumer) {
                    return g.tx().onReadWrite(consumer);
                }

                @Override
                public Transaction onReadWrite(Consumer<Transaction> consumer) {
                    return g.tx().onReadWrite(consumer);
                }

                @Override
                public void addTransactionListener(Consumer<Status> listener) {
                    g.tx().addTransactionListener(listener);
                }

                @Override
                public void commit() {
                    g.tx().commit();
                }

                @Override
                public void clearTransactionListeners() {
                    g.tx().clearTransactionListeners();

                }

                @Override
                public void rollback() {
                    g.tx().rollback();
                }

                @Override
                public boolean isOpen() {
                    return g.tx().isOpen();
                }

                @Override
                public void readWrite() {
                    g.tx().open();
                }

                @Override
                public void removeTransactionListener(Consumer<Status> listener) {
                    g.tx().removeTransactionListener(listener);
                }

                @Override
                public void close() {
                    g.tx().close();
                }
            });
        }
        return transaction.get();
    }

    @Override
    public Features features() {
        return new Features() {
            @Override
            public GraphFeatures graph() {
                return new GraphFeatures() {
                    @Override
                    public boolean supportsTransactions() {
                        return true;
                    }

                    @Override
                    public VariableFeatures variables() {
                        return new VariableFeatures() {
                            @Override
                            public boolean supportsVariables() {
                                return false;
                            }
                        };
                    }
                };
            }

            @Override
            public VertexFeatures vertex() {
                return new VertexFeatures() {
                    @Override
                    public boolean supportsUserSuppliedIds() {
                        return false;
                    }

                    @Override
                    public boolean supportsMultiProperties() {
                        return true;
                    }

                    @Override
                    public boolean supportsMetaProperties() {
                        return false;
                    }

                    @Override
                    public VertexPropertyFeatures properties() {
                        return new VertexPropertyFeatures() {
                            @Override
                            public boolean supportsProperties() {
                                return true;
                            }

                            @Override
                            public boolean supportsStringValues() {
                                return true;
                            }

                            @Override
                            public boolean supportsIntegerValues() {
                                return true;
                            }

                            @Override
                            public boolean supportsBooleanValues() {
                                return true;
                            }

                            @Override
                            public boolean supportsFloatValues() {
                                return true;
                            }

                            @Override
                            public boolean supportsDoubleValues() {
                                return true;
                            }

                            @Override
                            public boolean supportsLongValues() {
                                return true;
                            }

                            @Override
                            public boolean supportsMapValues() {
                                return false;
                            }
                        };
                    }
                };
            }

            @Override
            public EdgeFeatures edge() {
                return new EdgeFeatures() {
                    @Override
                    public boolean supportsUserSuppliedIds() {
                        return false;
                    }

                    @Override
                    public EdgePropertyFeatures properties() {
                        return new EdgePropertyFeatures() {
                            @Override
                            public boolean supportsProperties() {
                                return true;
                            }

                            @Override
                            public boolean supportsStringValues() {
                                return true;
                            }

                            @Override
                            public boolean supportsIntegerValues() {
                                return true;
                            }

                            @Override
                            public boolean supportsBooleanValues() {
                                return true;
                            }

                            @Override
                            public boolean supportsFloatValues() {
                                return true;
                            }

                            @Override
                            public boolean supportsDoubleValues() {
                                return true;
                            }

                            @Override
                            public boolean supportsLongValues() {
                                return true;
                            }

                            @Override
                            public boolean supportsMapValues() {
                                return false;
                            }
                        };
                    }
                };
            }
        };
    }

    @Override
    public Iterator<Vertex> vertices(Object... vertexIds) {
        if (vertexIds == null || vertexIds.length == 0) {
            return g.V();
        } else {
            return g.V(vertexIds);
        }
    }

    @Override
    public Iterator<Edge> edges(Object... edgeIds) {
        if (edgeIds == null || edgeIds.length == 0) {
            return g.E();
        } else {
            return g.E(edgeIds);
        }
    }
}

• My Supplier :

@Alternative
@Priority(Interceptor.Priority.APPLICATION)
public class NeptuneGraphSupplier implements Supplier<Graph> {

    @Produces
    @ApplicationScoped
    @Override
    public Graph get() {
        return new NeptuneGraph();
    }
}

• My Vertex class :

@Entity("test")
@Data
@Builder
@NoArgsConstructor
@AllArgsConstructor
public class Workspace {

    @Id
    private Long id;

    @Column("artifactType")
    private String artifactType;
}

3. Run the test

package be.ethias.cloud.models.vertex;

import java.util.Optional;

import org.eclipse.jnosql.databases.tinkerpop.mapping.TinkerpopTemplate;
import org.junit.jupiter.api.Test;
// import org.neo4j.ogm.session.Session;
// import org.neo4j.ogm.session.SessionFactory;

import be.ethias.cloud.models.property.commons.jnosql.Workspace;
import io.quarkus.test.junit.QuarkusTest;
import jakarta.inject.Inject;
import lombok.extern.slf4j.Slf4j;

@Slf4j
@QuarkusTest
public class VertexDaoImplTest {

    // -- Properties ---------------------------------------------------------

    @Inject
    TinkerpopTemplate graphTemplate;

    @Test
    void testPersist() {

        Workspace created = graphTemplate.insert(Workspace.builder().artifactType("test").id(1L).build());

        log.info("Workspace is [{}]", created);
    }
}

[otaviojava]

@dearrudam could you check this one, please?

[otaviojava]

@dearrudam is there any update on this?

[dearrudam]

Hi @otaviojava and @JulienDropsy ! Hope you both are doing well!

I’ve been testing the JNoSQL TinkerPop Database implementation and found that it works fine for embedded databases, but fails when using a remote one.

The TinkerPop docs recommend avoiding methods such as Graph.addVertex() or Vertex.properties(), and instead relying on Gremlin (g.addV(), g.V(1).properties(), etc.):

It is therefore recommended that users avoid usage of methods like Graph.addVertex() or Vertex.properties() and instead prefer use of Gremlin with g.addV() or g.V(1).properties().

Source: https://tinkerpop.apache.org/docs/3.7.4/reference/#staying-agnostic

Based on this, I think some adjustments are needed in the JNoSQL TinkerPop Database module.
I’m planning to start working on this soon — but let me know if you already have something in mind, or if you'd like me to open a dedicated issue/PR.

[JulienDropsy]

Hi @dearrudam, thanks for work. If the question is also for me, I have nothing in mind other then what i drafted. For clarification, I didn't use a Graph coming from TinkerGraph.open() and then proceed with already implemented Graph.addVertex(). If you look at the classes provided above, I made a kind of draft implementation of 'org.apache.tinkerpop.gremlin.structure.Graph' where addVertex method is implemented as follow

`public Vertex addVertex(Object... keyValues) {

    ElementHelper.legalPropertyKeyValueArray(keyValues);
    String label = ElementHelper.getLabelValue(keyValues).orElse(Vertex.DEFAULT_LABEL);
    if (label == null) {
        throw Graph.Exceptions.argumentCanNotBeNull("label");
    }

    List<Object> keyValuesList = List.of(keyValues);

    if (keyValuesList.size() < 2) {
        throw Graph.Exceptions.argumentCanNotBeNull("keyValues must contain at least a key and a value");
    }
    if (keyValuesList.size() % 2 != 0) {
        throw Graph.Exceptions.argumentCanNotBeNull("keyValues must contain an even number of elements");
    }

    return g.addV(label).next();
}`

And there, there is the call for the traversal 'g.addV(label).next();' which is not the same as the call you mention here I think :

Because i did not call TinkerGraph.open but i draft implemented a remote tinkerpop neptune graph connection with all features etc.

After that, others classes like GraphSupplier call my remote graph implementation (NeptuneGraph) that connects with TinkerpopTemplate.
The fact is that TinkerpopTemplate will in the call stack go through DefaultTinkerpopGraphDatabaseManager.insert where there is this line of code vertex = graph.addVertex(entity.name());. This line is not I think the line you mention Graph.addVertex because it is not coming from TinkerGraph.open but from my own implementation which does use recommended gremlin g.addV() :-)

Maybe my draft implementation is not so bad and could be a starting point. The minimal change that should be implemented is to bundle the call of these (in DefaultTinkerpopGraphDatabaseManager.insert)
Vertex vertex = graph.addVertex(entity.name()); entity.elements().forEach(e -> vertex.property(e.name(), ValueUtil.convert(e.value()))); entity.add(ID_PROPERTY, vertex.id()); vertex.property(ID_PROPERTY, vertex.id());
into one single call (maybe graph.addVertex(id, properties)) so that we have no more problem of detached vertex after the call of graph.addVertex that we can use anymore to "update" properties and others fields after vertex creation .

I hope it is 'understandable' because i am from Belgium (french speaking) ;-)