javeme commented on code in PR #2143:
URL: 
https://github.com/apache/incubator-hugegraph/pull/2143#discussion_r1126433234


##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherClient.java:
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hugegraph.api.cypher;
+
+import org.apache.hugegraph.util.E;
+import org.apache.hugegraph.util.Log;
+import org.apache.commons.configuration2.Configuration;
+import org.apache.tinkerpop.gremlin.driver.*;
+import org.apache.tinkerpop.gremlin.driver.message.RequestMessage;
+import org.slf4j.Logger;
+
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.ThreadSafe;
+
+import java.util.*;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Supplier;
+
+@ThreadSafe
+public final class CypherClient {
+    private static final Logger LOG = Log.logger(CypherClient.class);
+    private String userName;
+    private String password;
+    private String token;
+    private Supplier<Configuration> configurationSupplier;
+
+    CypherClient(String userName, String password,
+                 Supplier<Configuration> configurationSupplier) {
+        this.userName = userName;
+        this.password = password;
+        this.configurationSupplier = configurationSupplier;
+    }
+
+    CypherClient(String token, Supplier<Configuration> configurationSupplier) {
+        this.token = token;
+        this.configurationSupplier = configurationSupplier;
+    }
+
+    public CypherModel submitQuery(String cypherQuery, @Nullable Map<String, 
String> aliases) {
+        E.checkArgument(cypherQuery != null && !cypherQuery.isEmpty(),
+                        "The cypher-query parameter can't be null or empty");
+
+        Cluster cluster = Cluster.open(getConfig());
+        Client client = cluster.connect();
+
+        if (aliases != null && !aliases.isEmpty()) {
+            client = client.alias(aliases);
+        }
+
+        RequestMessage request = createRequest(cypherQuery);
+        CypherModel res = null;
+
+        try {
+            List<Object> list = this.doQueryList(client, request);
+            res = CypherModel.dataOf(request.getRequestId().toString(), list);
+        } catch (Exception e) {
+            LOG.error(String.format("Failed to submit cypher-query: [ %s ], 
cause by:"

Review Comment:
   we can use "{}" as placeholders in LOG message



##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherClient.java:
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hugegraph.api.cypher;
+
+import org.apache.hugegraph.util.E;
+import org.apache.hugegraph.util.Log;
+import org.apache.commons.configuration2.Configuration;
+import org.apache.tinkerpop.gremlin.driver.*;
+import org.apache.tinkerpop.gremlin.driver.message.RequestMessage;
+import org.slf4j.Logger;
+
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.ThreadSafe;
+
+import java.util.*;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Supplier;
+
+@ThreadSafe
+public final class CypherClient {
+    private static final Logger LOG = Log.logger(CypherClient.class);
+    private String userName;
+    private String password;
+    private String token;
+    private Supplier<Configuration> configurationSupplier;
+
+    CypherClient(String userName, String password,
+                 Supplier<Configuration> configurationSupplier) {
+        this.userName = userName;
+        this.password = password;
+        this.configurationSupplier = configurationSupplier;
+    }
+
+    CypherClient(String token, Supplier<Configuration> configurationSupplier) {
+        this.token = token;
+        this.configurationSupplier = configurationSupplier;
+    }
+
+    public CypherModel submitQuery(String cypherQuery, @Nullable Map<String, 
String> aliases) {
+        E.checkArgument(cypherQuery != null && !cypherQuery.isEmpty(),
+                        "The cypher-query parameter can't be null or empty");
+
+        Cluster cluster = Cluster.open(getConfig());
+        Client client = cluster.connect();
+
+        if (aliases != null && !aliases.isEmpty()) {
+            client = client.alias(aliases);
+        }
+
+        RequestMessage request = createRequest(cypherQuery);
+        CypherModel res = null;
+
+        try {
+            List<Object> list = this.doQueryList(client, request);
+            res = CypherModel.dataOf(request.getRequestId().toString(), list);
+        } catch (Exception e) {
+            LOG.error(String.format("Failed to submit cypher-query: [ %s ], 
cause by:"
+                , cypherQuery), e);
+            res = CypherModel.failOf(request.getRequestId().toString(), 
e.getMessage());
+        } finally {
+            client.close();
+            cluster.close();
+        }
+
+        return res;
+    }
+
+    private RequestMessage createRequest(String cypherQuery) {
+        return RequestMessage.build(Tokens.OPS_EVAL)
+                             .processor("cypher")
+                             .add(Tokens.ARGS_GREMLIN, cypherQuery)
+                             .create();
+    }
+
+    private List<Object> doQueryList(Client client, RequestMessage request)
+        throws ExecutionException, InterruptedException {
+
+        ResultSet results = null;
+        results = client.submitAsync(request).get();

Review Comment:
   it's more readable to keep in one line `ResultSet results = 
client.submitAsync(request).get();`



##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherAPI.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hugegraph.api.cypher;
+
+import jakarta.inject.Singleton;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.Produces;
+
+import org.apache.hugegraph.HugeException;
+import org.apache.hugegraph.api.API;
+import org.apache.hugegraph.api.filter.CompressInterceptor;
+import org.apache.hugegraph.util.E;
+import org.apache.hugegraph.util.Log;
+
+import com.codahale.metrics.annotation.Timed;
+
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.commons.lang3.tuple.Pair;
+import org.slf4j.Logger;
+
+import jakarta.ws.rs.Consumes;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.PathParam;
+import jakarta.ws.rs.QueryParam;
+import jakarta.ws.rs.core.Context;
+import jakarta.ws.rs.core.HttpHeaders;
+
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+import java.util.Base64;
+import java.util.HashMap;
+import java.util.Map;
+
+@Path("graphs/{graph}/cypher")
+@Singleton
+public class CypherAPI extends API {
+    private static final Logger LOG = Log.logger(CypherAPI.class);
+    private static final Charset UTF8 = 
Charset.forName(StandardCharsets.UTF_8.name());
+    private final Base64.Decoder decoder = Base64.getUrlDecoder();
+    private final String basic = "Basic ";
+    private final String bearer = "Bearer ";
+
+    private CypherManager cypherManager;
+
+    private CypherManager cypherManager() {
+        if (this.cypherManager == null) {
+            this.cypherManager = 
CypherManager.configOf("conf/remote-objects.yaml");
+        }
+        return this.cypherManager;
+    }
+
+    @GET
+    @Timed
+    @CompressInterceptor.Compress(buffer = (1024 * 40))
+    @Produces(APPLICATION_JSON_WITH_CHARSET)
+    public CypherModel query(@PathParam("graph") String graph, @Context 
HttpHeaders headers,
+                             @QueryParam("cypher") String cypher) {
+        LOG.debug("Graph [{}] query by cypher: {}", graph, cypher);
+
+        return this.queryByCypher(graph, headers, cypher);
+    }
+
+    @POST
+    @Timed
+    @CompressInterceptor.Compress
+    @Consumes(APPLICATION_JSON)
+    @Produces(APPLICATION_JSON_WITH_CHARSET)
+    public CypherModel post(@PathParam("graph") String graph, @Context 
HttpHeaders headers,
+                            String cypher) {
+        LOG.debug("Graph [{}] query by cypher: {}", graph, cypher);
+        return this.queryByCypher(graph, headers, cypher);
+    }
+
+    private CypherModel queryByCypher(String graph, HttpHeaders headers, 
String cypher) {
+
+        E.checkArgument(graph != null && !graph.isEmpty(),
+                        "The graph parameter can't be null or empty");
+        E.checkArgument(cypher != null && !cypher.isEmpty(),
+                        "The cypher parameter can't be null or empty");
+
+        Map<String, String> aliases = new HashMap<>(1, 1);
+        aliases.put("g", "__g_" + graph);
+
+        return this.client(headers).submitQuery(cypher, aliases);
+    }
+
+    private CypherClient client(HttpHeaders headers) {
+        String auth = headers.getHeaderString(HttpHeaders.AUTHORIZATION);
+
+        if (auth != null && !auth.isEmpty()) {
+            auth = auth.split(",")[0];
+        }
+
+        if (auth == null) {
+            throw new HugeException("The Cypher-API is being called without 
any authorization.");
+        }
+
+        if (auth.startsWith(basic)) {
+            return this.clientViaBasic(auth);
+        } else if (auth.startsWith(bearer)) {
+            return this.clientViaToken(auth);
+        }
+
+        throw new HugeException("The Cypher-API is being called without any 
authorization.");
+    }
+
+    private CypherClient clientViaBasic(String auth) {
+        Pair<String, String> userPass = this.toUserPass(auth);
+        E.checkNotNull(userPass, "user-password-pair");
+
+        return this.cypherManager().getClient(userPass.getLeft(), 
userPass.getRight());
+    }
+
+    private CypherClient clientViaToken(String auth) {
+        return this.cypherManager().getClient(auth.substring(bearer.length()));
+    }
+
+    private Pair<String, String> toUserPass(String auth) {
+        if (auth == null || auth.isEmpty()) return null;
+        if (!auth.startsWith(basic)) return null;
+
+        String[] split = null;
+
+        try {
+            String encoded = auth.substring(basic.length());
+            byte[] userPass = this.decoder.decode(encoded);
+            String authorization = new String(userPass, UTF8);
+            split = authorization.split(":");
+        } catch (Exception e) {
+            LOG.error("Failed convert auth to credential.", e);
+            return null;
+        }
+
+        if (split.length != 2) {
+            return null;
+        }
+
+        return ImmutablePair.of(split[0], split[1]);
+    }
+

Review Comment:
   can remove the blank line



##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherAPI.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hugegraph.api.cypher;
+
+import jakarta.inject.Singleton;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.Produces;
+
+import org.apache.hugegraph.HugeException;
+import org.apache.hugegraph.api.API;
+import org.apache.hugegraph.api.filter.CompressInterceptor;
+import org.apache.hugegraph.util.E;
+import org.apache.hugegraph.util.Log;
+
+import com.codahale.metrics.annotation.Timed;
+
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.commons.lang3.tuple.Pair;
+import org.slf4j.Logger;
+
+import jakarta.ws.rs.Consumes;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.PathParam;
+import jakarta.ws.rs.QueryParam;
+import jakarta.ws.rs.core.Context;
+import jakarta.ws.rs.core.HttpHeaders;
+
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+import java.util.Base64;
+import java.util.HashMap;
+import java.util.Map;
+
+@Path("graphs/{graph}/cypher")
+@Singleton
+public class CypherAPI extends API {
+    private static final Logger LOG = Log.logger(CypherAPI.class);
+    private static final Charset UTF8 = 
Charset.forName(StandardCharsets.UTF_8.name());
+    private final Base64.Decoder decoder = Base64.getUrlDecoder();
+    private final String basic = "Basic ";
+    private final String bearer = "Bearer ";
+
+    private CypherManager cypherManager;
+
+    private CypherManager cypherManager() {
+        if (this.cypherManager == null) {
+            this.cypherManager = 
CypherManager.configOf("conf/remote-objects.yaml");
+        }
+        return this.cypherManager;
+    }
+
+    @GET
+    @Timed
+    @CompressInterceptor.Compress(buffer = (1024 * 40))
+    @Produces(APPLICATION_JSON_WITH_CHARSET)
+    public CypherModel query(@PathParam("graph") String graph, @Context 
HttpHeaders headers,
+                             @QueryParam("cypher") String cypher) {
+        LOG.debug("Graph [{}] query by cypher: {}", graph, cypher);
+
+        return this.queryByCypher(graph, headers, cypher);
+    }
+
+    @POST
+    @Timed
+    @CompressInterceptor.Compress
+    @Consumes(APPLICATION_JSON)
+    @Produces(APPLICATION_JSON_WITH_CHARSET)
+    public CypherModel post(@PathParam("graph") String graph, @Context 
HttpHeaders headers,
+                            String cypher) {
+        LOG.debug("Graph [{}] query by cypher: {}", graph, cypher);
+        return this.queryByCypher(graph, headers, cypher);
+    }
+
+    private CypherModel queryByCypher(String graph, HttpHeaders headers, 
String cypher) {
+
+        E.checkArgument(graph != null && !graph.isEmpty(),
+                        "The graph parameter can't be null or empty");
+        E.checkArgument(cypher != null && !cypher.isEmpty(),
+                        "The cypher parameter can't be null or empty");
+
+        Map<String, String> aliases = new HashMap<>(1, 1);
+        aliases.put("g", "__g_" + graph);
+
+        return this.client(headers).submitQuery(cypher, aliases);
+    }
+
+    private CypherClient client(HttpHeaders headers) {
+        String auth = headers.getHeaderString(HttpHeaders.AUTHORIZATION);
+
+        if (auth != null && !auth.isEmpty()) {
+            auth = auth.split(",")[0];
+        }
+
+        if (auth == null) {
+            throw new HugeException("The Cypher-API is being called without 
any authorization.");
+        }
+
+        if (auth.startsWith(basic)) {
+            return this.clientViaBasic(auth);
+        } else if (auth.startsWith(bearer)) {
+            return this.clientViaToken(auth);
+        }
+
+        throw new HugeException("The Cypher-API is being called without any 
authorization.");

Review Comment:
   prefer to throw NotAuthorizedException instead



##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherAPI.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hugegraph.api.cypher;
+
+import jakarta.inject.Singleton;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.Produces;
+
+import org.apache.hugegraph.HugeException;
+import org.apache.hugegraph.api.API;
+import org.apache.hugegraph.api.filter.CompressInterceptor;
+import org.apache.hugegraph.util.E;
+import org.apache.hugegraph.util.Log;
+
+import com.codahale.metrics.annotation.Timed;
+
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.commons.lang3.tuple.Pair;
+import org.slf4j.Logger;
+
+import jakarta.ws.rs.Consumes;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.PathParam;
+import jakarta.ws.rs.QueryParam;
+import jakarta.ws.rs.core.Context;
+import jakarta.ws.rs.core.HttpHeaders;
+
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+import java.util.Base64;
+import java.util.HashMap;
+import java.util.Map;
+
+@Path("graphs/{graph}/cypher")
+@Singleton
+public class CypherAPI extends API {
+    private static final Logger LOG = Log.logger(CypherAPI.class);
+    private static final Charset UTF8 = 
Charset.forName(StandardCharsets.UTF_8.name());
+    private final Base64.Decoder decoder = Base64.getUrlDecoder();
+    private final String basic = "Basic ";
+    private final String bearer = "Bearer ";
+
+    private CypherManager cypherManager;
+
+    private CypherManager cypherManager() {
+        if (this.cypherManager == null) {
+            this.cypherManager = 
CypherManager.configOf("conf/remote-objects.yaml");

Review Comment:
   can we define a const var for "conf/remote-objects.yaml"



##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherAPI.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hugegraph.api.cypher;
+
+import jakarta.inject.Singleton;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.Produces;
+
+import org.apache.hugegraph.HugeException;
+import org.apache.hugegraph.api.API;
+import org.apache.hugegraph.api.filter.CompressInterceptor;
+import org.apache.hugegraph.util.E;
+import org.apache.hugegraph.util.Log;
+
+import com.codahale.metrics.annotation.Timed;
+
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.commons.lang3.tuple.Pair;
+import org.slf4j.Logger;
+
+import jakarta.ws.rs.Consumes;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.PathParam;
+import jakarta.ws.rs.QueryParam;
+import jakarta.ws.rs.core.Context;
+import jakarta.ws.rs.core.HttpHeaders;
+
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+import java.util.Base64;
+import java.util.HashMap;
+import java.util.Map;
+
+@Path("graphs/{graph}/cypher")
+@Singleton
+public class CypherAPI extends API {
+    private static final Logger LOG = Log.logger(CypherAPI.class);
+    private static final Charset UTF8 = 
Charset.forName(StandardCharsets.UTF_8.name());
+    private final Base64.Decoder decoder = Base64.getUrlDecoder();
+    private final String basic = "Basic ";
+    private final String bearer = "Bearer ";
+
+    private CypherManager cypherManager;
+
+    private CypherManager cypherManager() {
+        if (this.cypherManager == null) {
+            this.cypherManager = 
CypherManager.configOf("conf/remote-objects.yaml");
+        }
+        return this.cypherManager;
+    }
+
+    @GET
+    @Timed
+    @CompressInterceptor.Compress(buffer = (1024 * 40))
+    @Produces(APPLICATION_JSON_WITH_CHARSET)
+    public CypherModel query(@PathParam("graph") String graph, @Context 
HttpHeaders headers,
+                             @QueryParam("cypher") String cypher) {
+        LOG.debug("Graph [{}] query by cypher: {}", graph, cypher);
+
+        return this.queryByCypher(graph, headers, cypher);
+    }
+
+    @POST
+    @Timed
+    @CompressInterceptor.Compress
+    @Consumes(APPLICATION_JSON)
+    @Produces(APPLICATION_JSON_WITH_CHARSET)
+    public CypherModel post(@PathParam("graph") String graph, @Context 
HttpHeaders headers,
+                            String cypher) {
+        LOG.debug("Graph [{}] query by cypher: {}", graph, cypher);
+        return this.queryByCypher(graph, headers, cypher);
+    }
+
+    private CypherModel queryByCypher(String graph, HttpHeaders headers, 
String cypher) {
+
+        E.checkArgument(graph != null && !graph.isEmpty(),
+                        "The graph parameter can't be null or empty");
+        E.checkArgument(cypher != null && !cypher.isEmpty(),
+                        "The cypher parameter can't be null or empty");
+
+        Map<String, String> aliases = new HashMap<>(1, 1);
+        aliases.put("g", "__g_" + graph);
+
+        return this.client(headers).submitQuery(cypher, aliases);
+    }
+
+    private CypherClient client(HttpHeaders headers) {
+        String auth = headers.getHeaderString(HttpHeaders.AUTHORIZATION);
+
+        if (auth != null && !auth.isEmpty()) {
+            auth = auth.split(",")[0];
+        }
+
+        if (auth == null) {
+            throw new HugeException("The Cypher-API is being called without 
any authorization.");

Review Comment:
   prefer to keep one throw exit and update to:
   ```java
   if (auth != null) {
        if (auth.startsWith(basic)) {
           return this.clientViaBasic(auth);
       } else if (auth.startsWith(bearer)) {
           return this.clientViaToken(auth);
       }
   }
   throw ...
   ```



##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherClient.java:
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hugegraph.api.cypher;
+
+import org.apache.hugegraph.util.E;
+import org.apache.hugegraph.util.Log;
+import org.apache.commons.configuration2.Configuration;
+import org.apache.tinkerpop.gremlin.driver.*;
+import org.apache.tinkerpop.gremlin.driver.message.RequestMessage;
+import org.slf4j.Logger;
+
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.ThreadSafe;
+
+import java.util.*;

Review Comment:
   not allowed to import *



##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherPlugin.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2018-2019 "Neo4j, Inc." [https://neo4j.com]

Review Comment:
   also need to update the [LICENSE 
file](https://github.com/apache/incubator-hugegraph/blob/master/LICENSE) if the 
3rd-party source files are referenced.



##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherClient.java:
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hugegraph.api.cypher;
+
+import org.apache.hugegraph.util.E;
+import org.apache.hugegraph.util.Log;
+import org.apache.commons.configuration2.Configuration;
+import org.apache.tinkerpop.gremlin.driver.*;
+import org.apache.tinkerpop.gremlin.driver.message.RequestMessage;
+import org.slf4j.Logger;
+
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.ThreadSafe;
+
+import java.util.*;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Supplier;
+
+@ThreadSafe
+public final class CypherClient {

Review Comment:
   one blank line expected after the class define



##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherOpProcessor.java:
##########
@@ -0,0 +1,307 @@
+/*
+ * Copyright (c) 2018-2019 "Neo4j, Inc." [https://neo4j.com]
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+
+/**
+ * Description of the modifications:
+ * <p>
+ * 1) Changed the method signature to adopt the gremlin-server 3.5.1.
+ * public Optional<ThrowingConsumer<Context>> selectOther(RequestMessage 
requestMessage)
+ * -->
+ * public Optional<ThrowingConsumer<Context>> selectOther(Context ctx)
+ * <p>
+ * 2) Changed the package name.
+ * org.opencypher.gremlin.server.op.cypher
+ * -->
+ * org.apache.hugegraph.api.cypher
+ */
+
+package org.apache.hugegraph.api.cypher;
+
+import io.netty.channel.ChannelHandlerContext;
+
+import org.apache.tinkerpop.gremlin.driver.Tokens;
+import org.apache.tinkerpop.gremlin.driver.message.RequestMessage;
+import org.apache.tinkerpop.gremlin.driver.message.ResponseMessage;
+import org.apache.tinkerpop.gremlin.driver.message.ResponseStatusCode;
+import org.apache.tinkerpop.gremlin.process.traversal.P;
+import org.apache.tinkerpop.gremlin.process.traversal.TraversalSource;
+import 
org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.DefaultGraphTraversal;
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal;
+import 
org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
+import 
org.apache.tinkerpop.gremlin.process.traversal.util.TraversalInterruptedException;
+import org.apache.tinkerpop.gremlin.server.Context;
+import org.apache.tinkerpop.gremlin.server.GraphManager;
+import org.apache.tinkerpop.gremlin.server.OpProcessor;
+import org.apache.tinkerpop.gremlin.server.op.AbstractEvalOpProcessor;
+import org.apache.tinkerpop.gremlin.server.op.OpProcessorException;
+import org.apache.tinkerpop.gremlin.structure.Graph;
+import org.apache.tinkerpop.gremlin.util.function.ThrowingConsumer;
+import org.opencypher.gremlin.translation.CypherAst;
+import org.opencypher.gremlin.translation.groovy.GroovyPredicate;
+import org.opencypher.gremlin.translation.ir.TranslationWriter;
+import org.opencypher.gremlin.translation.ir.model.GremlinStep;
+import org.opencypher.gremlin.translation.translator.Translator;
+import org.opencypher.gremlin.traversal.ParameterNormalizer;
+import org.opencypher.gremlin.traversal.ProcedureContext;
+import org.opencypher.gremlin.traversal.ReturnNormalizer;
+import org.slf4j.Logger;
+
+import scala.collection.Seq;
+
+import java.util.*;
+import java.util.concurrent.Future;
+import java.util.concurrent.FutureTask;
+import java.util.concurrent.TimeUnit;
+
+import static java.util.Collections.emptyMap;
+import static java.util.Collections.singletonList;
+import static java.util.Optional.empty;
+import static 
org.apache.tinkerpop.gremlin.driver.message.ResponseStatusCode.SERVER_ERROR;
+import static org.opencypher.gremlin.translation.StatementOption.EXPLAIN;
+import static org.slf4j.LoggerFactory.getLogger;
+
+/**
+ * {@link OpProcessor} implementation for processing Cypher {@link 
RequestMessage}s:
+ * <pre>
+ * {
+ *   "requestId": "&lt;some UUID&gt;",
+ *   "op": "eval",
+ *   "processor": "cypher",
+ *   "args": { "gremlin": "&lt;CYPHER QUERY&gt;" }
+ * }
+ * </pre>
+ */
+public class CypherOpProcessor extends AbstractEvalOpProcessor {
+    private static final String DEFAULT_TRANSLATOR_DEFINITION =
+        "gremlin+cfog_server_extensions+inline_parameters";
+
+    private static final Logger logger = getLogger(CypherOpProcessor.class);
+
+    public CypherOpProcessor() {
+        super(true);
+    }
+
+    @Override
+    public String getName() {
+        return "cypher";
+    }
+
+    @Override
+    public ThrowingConsumer<Context> getEvalOp() {
+        return this::evalCypher;
+    }
+
+    @Override
+    public Optional<ThrowingConsumer<Context>> selectOther(Context ctx)
+        throws OpProcessorException {
+        return empty();
+    }
+
+    private void evalCypher(Context context) throws OpProcessorException {
+        Map<String, Object> args = context.getRequestMessage().getArgs();
+        String cypher = (String) args.get(Tokens.ARGS_GREMLIN);
+        logger.info("Cypher: {}", cypher.replaceAll("\n", " "));
+
+        GraphTraversalSource gts = traversal(context);
+        DefaultGraphTraversal g = new DefaultGraphTraversal(gts.clone());
+        Map<String, Object> parameters = 
ParameterNormalizer.normalize(getParameters(args));
+        ProcedureContext procedureContext = ProcedureContext.global();
+        CypherAst ast = CypherAst.parse(cypher, parameters, 
procedureContext.getSignatures());
+
+        String translatorDefinition = getTranslatorDefinition(context);
+
+        Translator<String, GroovyPredicate> stringTranslator =
+            Translator.builder()
+                      .gremlinGroovy()
+                      .build(translatorDefinition);
+
+        Translator<GraphTraversal, P> traversalTranslator =
+            Translator.builder()
+                      .traversal(g)
+                      .build(translatorDefinition);
+
+        Seq<GremlinStep> ir = ast.translate(stringTranslator.flavor(),
+                                            stringTranslator.features(),
+                                            procedureContext);
+
+        String gremlin = TranslationWriter.write(ir, stringTranslator, 
parameters);
+        logger.info("Gremlin: {}", gremlin);

Review Comment:
   don't log.info every statements, log.trace is ok



##########
hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/HugePrimaryKeyStrategy.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hugegraph.traversal.optimize;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Step;
+import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
+import 
org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy.ProviderOptimizationStrategy;
+import org.apache.tinkerpop.gremlin.process.traversal.step.Mutating;
+import 
org.apache.tinkerpop.gremlin.process.traversal.step.map.AddVertexStartStep;
+import 
org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.AddPropertyStep;
+import 
org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy;
+import org.apache.tinkerpop.gremlin.structure.T;
+import org.apache.tinkerpop.gremlin.process.traversal.step.map.AddVertexStep;
+import org.apache.tinkerpop.gremlin.structure.VertexProperty.Cardinality;
+
+import java.util.LinkedList;
+import java.util.List;
+
+

Review Comment:
   one blank line is ok



##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherOpProcessor.java:
##########
@@ -0,0 +1,307 @@
+/*
+ * Copyright (c) 2018-2019 "Neo4j, Inc." [https://neo4j.com]
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+
+/**
+ * Description of the modifications:
+ * <p>
+ * 1) Changed the method signature to adopt the gremlin-server 3.5.1.
+ * public Optional<ThrowingConsumer<Context>> selectOther(RequestMessage 
requestMessage)
+ * -->
+ * public Optional<ThrowingConsumer<Context>> selectOther(Context ctx)
+ * <p>
+ * 2) Changed the package name.
+ * org.opencypher.gremlin.server.op.cypher

Review Comment:
   can we just import it, or move this reference files to an independent 
package like `org.apache.hugegraph.api.cypher.opencypher`?



##########
hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/HugePrimaryKeyStrategy.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hugegraph.traversal.optimize;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Step;
+import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
+import 
org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy.ProviderOptimizationStrategy;
+import org.apache.tinkerpop.gremlin.process.traversal.step.Mutating;
+import 
org.apache.tinkerpop.gremlin.process.traversal.step.map.AddVertexStartStep;
+import 
org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.AddPropertyStep;
+import 
org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy;
+import org.apache.tinkerpop.gremlin.structure.T;
+import org.apache.tinkerpop.gremlin.process.traversal.step.map.AddVertexStep;
+import org.apache.tinkerpop.gremlin.structure.VertexProperty.Cardinality;
+
+import java.util.LinkedList;
+import java.util.List;
+
+
+public class HugePrimaryKeyStrategy
+    extends 
AbstractTraversalStrategy<TraversalStrategy.ProviderOptimizationStrategy>
+    implements ProviderOptimizationStrategy {
+
+    private static final long serialVersionUID = 6307847098226016416L;
+    private static final HugePrimaryKeyStrategy INSTANCE = new 
HugePrimaryKeyStrategy();
+
+    public static HugePrimaryKeyStrategy instance() {
+        return INSTANCE;
+    }
+
+    @Override
+    public void apply(Traversal.Admin<?, ?> traversal) {
+
+        List<Step> removeSteps = new LinkedList<>();
+        Mutating curAddStep = null;
+        List<Step> stepList = traversal.getSteps();
+
+        for (int i = 0, s = stepList.size(); i < s; i++) {
+            Step step = stepList.get(i);
+
+            if (i == 0 && AddVertexStartStep.class.isInstance(step)) {
+                curAddStep = (Mutating) step;
+                continue;
+            } else if (curAddStep == null && 
AddVertexStep.class.isInstance((step))) {
+                curAddStep = (Mutating) step;
+                continue;
+            }
+
+            if (curAddStep == null) continue;
+
+            if (!AddPropertyStep.class.isInstance(step)) {
+                curAddStep = null;
+                continue;
+            }
+
+            AddPropertyStep propertyStep = (AddPropertyStep) step;
+
+            if (propertyStep.getCardinality() == Cardinality.single
+                || propertyStep.getCardinality() == null) {
+
+                Object[] kvs = new Object[2];
+                List<Object> kvList = new LinkedList<>();
+
+                propertyStep.getParameters().getRaw().forEach((k, v) -> {
+                    if (T.key.equals(k)) {
+                        kvs[0] = v.get(0);
+                    } else if (T.value.equals(k)) {
+                        kvs[1] = v.get(0);
+                    } else {
+                        kvList.add(k.toString());
+                        kvList.add(v.get(0));
+                    }
+                });
+
+                curAddStep.configure(kvs);
+
+                if (kvList.size() > 0) {
+                    curAddStep.configure(kvList.toArray(new 
Object[kvList.size()]));
+                }
+
+                removeSteps.add(step);
+            } else {
+                curAddStep = null;
+            }
+
+        }
+
+        for (Step index : removeSteps) {
+            traversal.removeStep(index);
+        }
+

Review Comment:
   unexpected blank line



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to