valepakh commented on code in PR #2339:
URL: https://github.com/apache/ignite-3/pull/2339#discussion_r1289810372
##########
modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/connect/ItConnectCommandTest.java:
##########
@@ -135,6 +136,28 @@ void connectTwice() {
assertThat(promptAfter).isEqualTo("[" + nodeName() + "]> ");
}
+ @Test
+ @DisplayName("Should throw error if cluster without authentication but
command invoked with username/password")
+ void clusterWithoutAuthUsernameButPasswordProvided() throws IOException {
Review Comment:
```suggestion
void clusterWithoutAuthButUsernameAndPasswordProvided() throws
IOException {
```
##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/call/connect/ConnectCallInput.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * 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.ignite.internal.cli.call.connect;
+
+import javax.annotation.Nullable;
Review Comment:
```suggestion
import org.jetbrains.annotations.Nullable;
```
##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/call/connect/ConnectCallInput.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * 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.ignite.internal.cli.call.connect;
+
+import javax.annotation.Nullable;
+import org.apache.ignite.internal.cli.core.call.CallInput;
+
+/** Input for the {@link ConnectCall} call. */
+public class ConnectCallInput implements CallInput {
+
+ private final String url;
+ @Nullable
+ private final String username;
+ @Nullable
+ private final String password;
Review Comment:
```suggestion
private final String url;
@Nullable
private final String username;
@Nullable
private final String password;
```
##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/connect/ConnectReplCommand.java:
##########
@@ -50,10 +55,29 @@ public class ConnectReplCommand extends BaseCommand
implements Runnable {
@Override
public void run() {
question.askQuestionIfConnected(nodeNameOrUrl.stringUrl())
- .map(UrlCallInput::new)
+ .map(url -> connectCallInput())
Review Comment:
Why didn't you pass the url to the method?
##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/core/repl/SessionInfo.java:
##########
@@ -17,6 +17,9 @@
package org.apache.ignite.internal.cli.core.repl;
+
+import javax.annotation.Nullable;
Review Comment:
```suggestion
import org.jetbrains.annotations.Nullable;
```
##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/call/connect/ConnectCallInput.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * 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.ignite.internal.cli.call.connect;
+
+import javax.annotation.Nullable;
+import org.apache.ignite.internal.cli.core.call.CallInput;
+
+/** Input for the {@link ConnectCall} call. */
+public class ConnectCallInput implements CallInput {
+
+ private final String url;
+ @Nullable
+ private final String username;
+ @Nullable
+ private final String password;
+
+ private ConnectCallInput(String url, @Nullable String username, @Nullable
String password) {
+ this.url = url;
+ this.username = username;
+ this.password = password;
+ }
+
+ String url() {
+ return url;
+ }
+
+ /**
+ * Provided username.
+ *
+ * @return username
+ */
+ @Nullable
+ String username() {
+ return username;
+ }
+
+ /**
+ * Provided password.
+ *
+ * @return password
+ */
+ @Nullable
+ String password() {
+ return password;
+ }
+
+ /**
+ * Builder method provider.
+ *
+ * @return new instance of {@link ConnectCallInputBuilder}.
+ */
+ public static ConnectCallInputBuilder builder() {
+ return new ConnectCallInputBuilder();
+ }
+
+ /** Builder for {@link ConnectCallInput}. */
+ public static class ConnectCallInputBuilder {
+
+ private String url;
+ @Nullable
Review Comment:
```suggestion
@Nullable
```
##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/questions/ConnectToClusterQuestion.java:
##########
@@ -17,17 +17,21 @@
package org.apache.ignite.internal.cli.commands.questions;
+import static
org.apache.ignite.internal.cli.config.CliConfigKeys.BASIC_AUTHENTICATION_PASSWORD;
+import static
org.apache.ignite.internal.cli.config.CliConfigKeys.BASIC_AUTHENTICATION_USERNAME;
import static
org.apache.ignite.internal.cli.core.style.component.QuestionUiComponent.fromYesNoQuestion;
+import static org.apache.ignite.lang.util.StringUtils.nullOrBlank;
import jakarta.inject.Inject;
import jakarta.inject.Singleton;
import java.util.Objects;
+import org.apache.ignite.internal.cli.call.connect.ConnectCallInput;
import org.apache.ignite.internal.cli.call.connect.ConnectSslCall;
import org.apache.ignite.internal.cli.call.connect.SslConfig;
import org.apache.ignite.internal.cli.config.CliConfigKeys;
import org.apache.ignite.internal.cli.config.ConfigManagerProvider;
import org.apache.ignite.internal.cli.config.StateConfigProvider;
-import org.apache.ignite.internal.cli.core.call.UrlCallInput;
+import org.apache.ignite.internal.cli.core.flow.Flowable;
Review Comment:
```suggestion
```
##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/questions/ConnectToClusterQuestion.java:
##########
@@ -109,6 +114,29 @@ public FlowBuilder<Void, String>
askQuestionIfConnected(String nodeUrl) {
return Flows.from(nodeUrl);
}
+ /**
+ * Ask if the user wants to store credentials in config.
+ *
+ * @param username username.
+ * @param password password
+ */
+ public void askQuestionToStoreCredentials(@Nullable String username,
@Nullable String password) {
+ if (!nullOrBlank(username) && !nullOrBlank(password)) {
+ String storedUsername =
configManagerProvider.get().getCurrentProperty(BASIC_AUTHENTICATION_USERNAME.value());
+ String storedPassword =
configManagerProvider.get().getCurrentProperty(BASIC_AUTHENTICATION_PASSWORD.value());
+
+ //Ask question only if cli config has different values
+ if (!username.equals(storedUsername) ||
!password.equals(storedPassword)) {
+ QuestionUiComponent question = fromYesNoQuestion("Remember
current credentials?");
+ Flows.acceptQuestion(question, () -> {
+
configManagerProvider.get().setProperty(BASIC_AUTHENTICATION_USERNAME.value(),
username);
+
configManagerProvider.get().setProperty(BASIC_AUTHENTICATION_PASSWORD.value(),
password);
+ return "Config saved";
+ }).print().build().start(Flowable.empty());
Review Comment:
```suggestion
}).print().start();
```
##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/connect/ConnectOptions.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * 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.ignite.internal.cli.commands.connect;
+
+import static
org.apache.ignite.internal.cli.commands.Options.Constants.PASSWORD_OPTION;
+import static
org.apache.ignite.internal.cli.commands.Options.Constants.PASSWORD_OPTION_DESC;
+import static
org.apache.ignite.internal.cli.commands.Options.Constants.PASSWORD_OPTION_SHORT;
+import static
org.apache.ignite.internal.cli.commands.Options.Constants.USERNAME_OPTION;
+import static
org.apache.ignite.internal.cli.commands.Options.Constants.USERNAME_OPTION_DESC;
+import static
org.apache.ignite.internal.cli.commands.Options.Constants.USERNAME_OPTION_SHORT;
+
+import org.jetbrains.annotations.Nullable;
+import picocli.CommandLine.Option;
+
+/**
+ * Mixin class for connect command options.
+ */
+public class ConnectOptions {
+
+ @Option(names = {USERNAME_OPTION,
+ USERNAME_OPTION_SHORT}, description = USERNAME_OPTION_DESC,
required = true)
+ private String username;
+
+ @Option(names = {PASSWORD_OPTION,
+ PASSWORD_OPTION_SHORT}, description = PASSWORD_OPTION_DESC,
required = true)
+ private String password;
+
+ @Nullable
Review Comment:
I don't think that a required option could be null.
--
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]
