javeme commented on code in PR #2143: URL: https://github.com/apache/incubator-hugegraph/pull/2143#discussion_r1132306890
########## hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherManager.java: ########## @@ -0,0 +1,107 @@ +/* + * 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 java.io.File; +import java.io.FileReader; +import java.io.Reader; +import java.net.URL; + +import javax.annotation.concurrent.ThreadSafe; + +import org.apache.commons.configuration2.Configuration; +import org.apache.commons.configuration2.YAMLConfiguration; +import org.apache.hugegraph.util.E; + +@ThreadSafe +public final class CypherManager { Review Comment: missing blank line after the class define ########## hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherManager.java: ########## @@ -0,0 +1,107 @@ +/* + * 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 java.io.File; +import java.io.FileReader; +import java.io.Reader; +import java.net.URL; + +import javax.annotation.concurrent.ThreadSafe; + +import org.apache.commons.configuration2.Configuration; +import org.apache.commons.configuration2.YAMLConfiguration; +import org.apache.hugegraph.util.E; + +@ThreadSafe +public final class CypherManager { + private final String configurationFile; + private YAMLConfiguration configuration; + + public static CypherManager configOf(String configurationFile) { + E.checkArgument(configurationFile != null && !configurationFile.isEmpty(), + "The configurationFile parameter can't be null or empty"); + return new CypherManager(configurationFile); + } + + private CypherManager(String configurationFile) { + this.configurationFile = configurationFile; + } + + public CypherClient getClient(String userName, String password) { + E.checkArgument(userName != null && !userName.isEmpty(), + "The userName parameter can't be null or empty"); + E.checkArgument(password != null && !password.isEmpty(), + "The password parameter can't be null or empty"); + + //TODO: Need to cache the client and make it hold the connection. Review Comment: expect a space after the "//" ########## hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherManager.java: ########## @@ -0,0 +1,107 @@ +/* + * 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 java.io.File; +import java.io.FileReader; +import java.io.Reader; +import java.net.URL; + +import javax.annotation.concurrent.ThreadSafe; + +import org.apache.commons.configuration2.Configuration; +import org.apache.commons.configuration2.YAMLConfiguration; +import org.apache.hugegraph.util.E; + +@ThreadSafe +public final class CypherManager { + private final String configurationFile; + private YAMLConfiguration configuration; + + public static CypherManager configOf(String configurationFile) { + E.checkArgument(configurationFile != null && !configurationFile.isEmpty(), + "The configurationFile parameter can't be null or empty"); + return new CypherManager(configurationFile); + } + + private CypherManager(String configurationFile) { + this.configurationFile = configurationFile; + } + + public CypherClient getClient(String userName, String password) { + E.checkArgument(userName != null && !userName.isEmpty(), + "The userName parameter can't be null or empty"); + E.checkArgument(password != null && !password.isEmpty(), + "The password parameter can't be null or empty"); + + //TODO: Need to cache the client and make it hold the connection. + return new CypherClient(userName, password, this::cloneConfig); + } + + public CypherClient getClient(String token) { + E.checkArgument(token != null && !token.isEmpty(), + "The token parameter can't be null or empty"); + + //TODO: Need to cache the client and make it hold the connection. + return new CypherClient(token, this::cloneConfig); + } + + private Configuration cloneConfig() { + if (this.configuration == null) { + this.configuration = loadYaml(this.configurationFile); + } + + return (Configuration) this.configuration.clone(); + } + + private static YAMLConfiguration loadYaml(String configurationFile) { + File yamlFile = getConfigFile(configurationFile); + YAMLConfiguration yaml; + + try { + Reader reader = new FileReader(yamlFile); + yaml = new YAMLConfiguration(); + yaml.read(reader); + } catch (Exception e) { + throw new RuntimeException(String.format("Failed to load configuration file," + + " the file at '%s'.", configurationFile), e); + } + + return yaml; + } + + private static File getConfigFile(String configurationFile) { + final File systemFile = new File(configurationFile); Review Comment: can we also remove these final mark? ########## hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherManager.java: ########## @@ -0,0 +1,107 @@ +/* + * 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 java.io.File; +import java.io.FileReader; +import java.io.Reader; +import java.net.URL; + +import javax.annotation.concurrent.ThreadSafe; + +import org.apache.commons.configuration2.Configuration; +import org.apache.commons.configuration2.YAMLConfiguration; +import org.apache.hugegraph.util.E; + +@ThreadSafe +public final class CypherManager { + private final String configurationFile; + private YAMLConfiguration configuration; + + public static CypherManager configOf(String configurationFile) { + E.checkArgument(configurationFile != null && !configurationFile.isEmpty(), + "The configurationFile parameter can't be null or empty"); + return new CypherManager(configurationFile); + } + + private CypherManager(String configurationFile) { + this.configurationFile = configurationFile; + } + + public CypherClient getClient(String userName, String password) { + E.checkArgument(userName != null && !userName.isEmpty(), + "The userName parameter can't be null or empty"); + E.checkArgument(password != null && !password.isEmpty(), + "The password parameter can't be null or empty"); + + //TODO: Need to cache the client and make it hold the connection. + return new CypherClient(userName, password, this::cloneConfig); + } + + public CypherClient getClient(String token) { + E.checkArgument(token != null && !token.isEmpty(), + "The token parameter can't be null or empty"); + + //TODO: Need to cache the client and make it hold the connection. Review Comment: ditto ########## hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java: ########## @@ -210,4 +214,76 @@ public static void initAdminUserIfNeeded(String confFile) throws Exception { auth.initAdminUser(); } } + + private class TokenSaslAuthenticator implements SaslNegotiator { + private static final byte NUL = 0; + private String username; + private String password; + private String token; + + @Override + public byte[] evaluateResponse(final byte[] clientResponse) throws AuthenticationException { + decode(clientResponse); + return null; + } + + @Override + public boolean isComplete() { + return this.username != null; + } + + @Override + public AuthenticatedUser getAuthenticatedUser() throws AuthenticationException { + if (!this.isComplete()) { + throw new AuthenticationException( + "The SASL negotiation has not yet been completed."); + } + + final Map<String, String> credentials = new HashMap<>(6, 1); + credentials.put(KEY_USERNAME, username); + credentials.put(KEY_PASSWORD, password); + credentials.put(KEY_TOKEN, token); + + return authenticate(credentials); + } + + /** + * SASL PLAIN mechanism specifies that credentials are encoded in a + * sequence of UTF-8 bytes, delimited by 0 (US-ASCII NUL). + * The form is : {code}authzId<NUL>authnId<NUL>password<NUL>{code}. + * + * @param bytes encoded credentials string sent by the client + */ + private void decode(byte[] bytes) throws AuthenticationException { + this.username = null; + this.password = null; + + int end = bytes.length; + + for (int i = bytes.length - 1; i >= 0; i--) { + if (bytes[i] == NUL) { Review Comment: prefer `continue if (bytes[i] != NUL)` to reduce the indentation ########## hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java: ########## @@ -210,4 +214,76 @@ public static void initAdminUserIfNeeded(String confFile) throws Exception { auth.initAdminUser(); } } + + private class TokenSaslAuthenticator implements SaslNegotiator { + private static final byte NUL = 0; Review Comment: expect a blank line ########## hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherClient.java: ########## @@ -81,21 +87,19 @@ public CypherModel submitQuery(String cypherQuery,@Nullable Map<String, String> private RequestMessage createRequest(String cypherQuery) { return RequestMessage.build(Tokens.OPS_EVAL) - .processor("cypher") - .add(Tokens.ARGS_GREMLIN, cypherQuery) - .create(); + .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(); + throws ExecutionException, InterruptedException { + ResultSet results = client.submitAsync(request).get(); Iterator<Result> iter = results.iterator(); List<Object> list = new LinkedList<>(); - for (; iter.hasNext(); ) { + while (iter.hasNext()) { Review Comment: LinkedList is not a memory-friendly structure, it generates a lot of extra objects. And in most scenarios, the performance of ArrayList is better, you can try to test them. -- 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]
