IGNITE-22781 Fix question during connect to a node

modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/CliIntegrationTest.java

@@ -113,7 +113,9 @@ void setUp() {
         configManagerProvider.setConfigFile(TestConfigManagerHelper.createIntegrationTestsConfig());
         cmd = new CommandLine(getCommandClass(), new MicronautFactory(context));
         cmd.setDefaultValueProvider(configDefaultValueProvider);
-        eventListeningActivationPoint.subscribe();
+        if (needToSubscribe()) {
+            eventListeningActivationPoint.subscribe();
+        }
         resetOutput();
         CommandLineContextProvider.setCmd(cmd);
     }
@@ -134,6 +136,16 @@ protected Class<?> getCommandClass() {
         return TopLevelCliCommand.class;
     }
 
+    /**
+     * If {@code true}, the test subscribes to CLI event. Return {@code false} to more closely emulate real CLI app, where we subscribe
+     * explicitly before the REPL mode start.
+     *
+     * @return Whether to subscribe to CLI events or not.
+     */
+    protected boolean needToSubscribe() {
+        return true;
+    }
+
     protected void execute(String... args) {
         resetOutput();
         exitCode = cmd.execute(args);

modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/ItConnectToClusterTestBase.java

@@ -17,9 +17,10 @@
 
 package org.apache.ignite.internal.cli.commands;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
 import jakarta.inject.Inject;
-import java.io.FileDescriptor;
-import java.io.FileOutputStream;
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -41,6 +42,7 @@
 public class ItConnectToClusterTestBase extends CliIntegrationTest {
     @Inject
     protected TestStateConfigProvider stateConfigProvider;
+
     @Inject
     protected ConnectToClusterQuestion question;
 
@@ -51,6 +53,8 @@ public class ItConnectToClusterTestBase extends CliIntegrationTest {
 
     private Path input;
 
+    private ByteArrayOutputStream output;
+
     @Override
     protected Class<?> getCommandClass() {
         return TopLevelCliReplCommand.class;
@@ -59,7 +63,8 @@ protected Class<?> getCommandClass() {
     @BeforeEach
     public void setUpTerminal() throws Exception {
         input = Files.createTempFile(WORK_DIR, "input", "");
-        terminal = new DumbTerminal(Files.newInputStream(input), new FileOutputStream(FileDescriptor.out));
+        output = new ByteArrayOutputStream();
+        terminal = new DumbTerminal(Files.newInputStream(input), output);
         QuestionAskerFactory.setWriterReaderFactory(new JlineQuestionWriterReaderFactory(terminal));
     }
 
@@ -73,11 +78,17 @@ protected String getPrompt() {
         return Ansi.OFF.string(promptProvider.getPrompt());
     }
 
-    protected String nodeName() {
+    protected static String nodeName() {
         return CLUSTER.node(0).name();
     }
 
     protected void bindAnswers(String... answers) throws IOException {
         Files.writeString(input, String.join("\n", answers) + "\n");
     }
+
+    protected void assertTerminalOutputIsEmpty() {
+        assertThat(output.toString())
+                .as("Expected terminal output to be empty")
+                .isEmpty();
+    }
 }

modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/connect/ItConnectCommandTest.java

@@ -17,13 +17,22 @@
 
 package org.apache.ignite.internal.cli.commands.connect;
 
+import static org.apache.ignite.internal.cli.commands.cliconfig.TestConfigManagerHelper.createEmptySecretConfig;
+import static org.apache.ignite.internal.cli.commands.cliconfig.TestConfigManagerHelper.createIntegrationTestsConfig;
+import static org.apache.ignite.internal.cli.commands.cliconfig.TestConfigManagerHelper.createJdbcTestsBasicSecretConfig;
+import static org.apache.ignite.internal.cli.commands.cliconfig.TestConfigManagerHelper.createNonExistingSecretConfig;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertAll;
 
-import java.io.IOException;
+import java.io.File;
+import java.util.List;
 import org.apache.ignite.internal.cli.commands.ItConnectToClusterTestBase;
 import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Named;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 class ItConnectCommandTest extends ItConnectToClusterTestBase {
 
@@ -39,7 +48,8 @@ void connectWithDefaultUrl() {
         // Then
         assertAll(
                 this::assertErrOutputIsEmpty,
-                () -> assertOutputContains("Connected to http://localhost:10300")
+                () -> assertOutputContains("Connected to http://localhost:10300"),
+                this::assertTerminalOutputIsEmpty
         );
         // And prompt is changed to connect
         assertThat(getPrompt()).isEqualTo("[" + nodeName() + "]> ");
@@ -54,7 +64,8 @@ void connectWithGivenUrl() {
         // Then
         assertAll(
                 this::assertErrOutputIsEmpty,
-                () -> assertOutputContains("Connected to http://localhost:10301")
+                () -> assertOutputContains("Connected to http://localhost:10301"),
+                this::assertTerminalOutputIsEmpty
         );
     }
 
@@ -67,7 +78,8 @@ void connectWithWrongUrl() {
         // Then
         assertAll(
                 () -> assertErrOutputIs("Node unavailable" + System.lineSeparator()
-                        + "Could not connect to node with URL http://localhost:11111" + System.lineSeparator())
+                        + "Could not connect to node with URL http://localhost:11111" + System.lineSeparator()),
+                this::assertTerminalOutputIsEmpty
         );
         // And prompt is
         assertThat(getPrompt()).isEqualTo("[disconnected]> ");
@@ -86,15 +98,28 @@ void disconnect() {
         // Then
         assertAll(
                 this::assertErrOutputIsEmpty,
-                () -> assertOutputContains("Disconnected from http://localhost:10300")
+                () -> assertOutputContains("Disconnected from http://localhost:10300"),
+                this::assertTerminalOutputIsEmpty
         );
         // And prompt is changed
         assertThat(getPrompt()).isEqualTo("[disconnected]> ");
     }
 
-    @Test
+    private static List<Arguments> secretConfigs() {
+        return List.of(
+                Arguments.of(Named.of("createEmptySecretConfig", createEmptySecretConfig())),
+                Arguments.of(Named.of("createNonExistingSecretConfig", createNonExistingSecretConfig())),
+                Arguments.of(Named.of("createJdbcTestsBasicSecretConfig", createJdbcTestsBasicSecretConfig()))
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("secretConfigs")
     @DisplayName("Should state that already connected")
-    void connectTwice() {
+    void connectTwice(File secretConfig) {
+        // Given secret config
+        configManagerProvider.setConfigFile(createIntegrationTestsConfig(), secretConfig);
+
         // Given connected to cluster
         execute("connect");
         // And output is
@@ -111,15 +136,16 @@ void connectTwice() {
         // Then
         assertAll(
                 this::assertErrOutputIsEmpty,
-                () -> assertOutputIs("You are already connected to http://localhost:10300" + System.lineSeparator())
+                () -> assertOutputIs("You are already connected to http://localhost:10300" + System.lineSeparator()),
+                this::assertTerminalOutputIsEmpty
         );
         // And prompt is still connected
         assertThat(getPrompt()).isEqualTo("[" + nodeName() + "]> ");
     }
 
     @Test
     @DisplayName("Should throw error if cluster without authentication but command invoked with username/password")
-    void clusterWithoutAuthButUsernamePasswordProvided() throws IOException {
+    void clusterWithoutAuthButUsernamePasswordProvided() {
 
         // Given prompt before connect
         assertThat(getPrompt()).isEqualTo("[disconnected]> ");
@@ -129,8 +155,10 @@ void clusterWithoutAuthButUsernamePasswordProvided() throws IOException {
 
         // Then
         assertAll(
+                this::assertOutputIsEmpty,
                 () -> assertErrOutputIs("Authentication is not enabled on cluster but username or password were provided."
-                        + System.lineSeparator())
+                        + System.lineSeparator()),
+                this::assertTerminalOutputIsEmpty
         );
 
         // And prompt is

modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/connect/ItConnectNonReplCommandTest.java

@@ -0,0 +1,64 @@
+/*
+ * 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 io.micronaut.context.annotation.Bean;
+import io.micronaut.context.annotation.Replaces;
+import jakarta.inject.Inject;
+import org.apache.ignite.internal.cli.ReplManager;
+import org.apache.ignite.internal.cli.commands.TopLevelCliCommand;
+import org.apache.ignite.internal.cli.core.repl.EventListeningActivationPoint;
+import org.junit.jupiter.api.Test;
+
+class ItConnectNonReplCommandTest extends ItConnectCommandTest {
+    @Inject
+    private EventListeningActivationPoint eventListeningActivationPoint;
+
+    @Bean
+    @Replaces(ReplManager.class)
+    public ReplManager replManager() {
+        return new ReplManager() {
+            @Override
+            public void subscribe() {
+                eventListeningActivationPoint.subscribe();
+            }
+
+            @Override
+            public void startReplMode() {
+                // Emulate repl start by asking a question.
+                question.askQuestionOnReplStart();
+            }
+        };
+    }
+
+    @Override
+    protected Class<?> getCommandClass() {
+        return TopLevelCliCommand.class;
+    }
+
+    @Override
+    protected boolean needToSubscribe() {
+        return false;
+    }
+
+    @Override
+    @Test
+    void disconnect() {
+        // Not implemented.
+    }
+}

modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/sql/ItSqlReplCommandNotInitialedClusterTest.java → modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/sql/ItSqlReplCommandNotInitializedClusterTest.java

@@ -24,14 +24,15 @@
 import org.apache.ignite.IgniteServer;
 import org.apache.ignite.InitParameters;
 import org.apache.ignite.internal.cli.CliIntegrationTest;
+import org.apache.ignite.internal.cli.commands.TopLevelCliReplCommand;
 import org.apache.ignite.internal.cli.core.repl.Session;
 import org.apache.ignite.internal.cli.core.repl.SessionInfo;
 import org.apache.ignite.internal.cli.core.repl.executor.ReplExecutorProvider;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 
 /** Tests for {@link SqlReplCommand} with not initialized cluster. */
-public class ItSqlReplCommandNotInitialedClusterTest extends CliIntegrationTest {
+public class ItSqlReplCommandNotInitializedClusterTest extends CliIntegrationTest {
     private final Session session = new Session();
 
     @Bean
@@ -46,6 +47,11 @@ public Session session() {
         return session;
     }
 
+    @Override
+    protected Class<?> getCommandClass() {
+        return TopLevelCliReplCommand.class;
+    }
+
     @Override
     protected boolean needInitializeCluster() {
         return false;

modules/cli/src/main/java/org/apache/ignite/internal/cli/Main.java

@@ -88,6 +88,7 @@ private static void enterRepl(MicronautFactory micronautFactory) throws Exceptio
         System.out.println(banner(versionProvider));
 
         ReplManager replManager = micronautFactory.create(ReplManager.class);
+        replManager.subscribe();
         replManager.startReplMode();
     }
 

modules/cli/src/main/java/org/apache/ignite/internal/cli/ReplManager.java

@@ -50,6 +50,13 @@ public class ReplManager {
     @Inject
     private EventListeningActivationPoint eventListeningActivationPoint;
 
+    /**
+     * Subscribes to CLI events. Should be called before {@link #startReplMode()}.
+     */
+    public void subscribe() {
+        eventListeningActivationPoint.subscribe();
+    }
+
     /**
      * Enters REPL mode.
      */
@@ -69,7 +76,6 @@ public void startReplMode() {
                 .withOnStart(question::askQuestionOnReplStart)
                 .withHistoryFileName("history")
                 .withAutosuggestionsWidgets()
-                .withEventSubscriber(eventListeningActivationPoint)
                 .build());
     }
 }

modules/cli/src/main/java/org/apache/ignite/internal/cli/call/connect/ConnectCall.java

@@ -18,12 +18,10 @@
 package org.apache.ignite.internal.cli.call.connect;
 
 import static org.apache.ignite.internal.cli.commands.questions.ConnectToClusterQuestion.askQuestionToStoreCredentials;
-import static org.apache.ignite.internal.cli.config.CliConfigKeys.BASIC_AUTHENTICATION_USERNAME;
 import static org.apache.ignite.internal.util.StringUtils.nullOrBlank;
 
 import io.micronaut.http.HttpStatus;
 import jakarta.inject.Singleton;
-import java.util.Objects;
 import org.apache.ignite.internal.cli.config.ConfigManagerProvider;
 import org.apache.ignite.internal.cli.core.call.Call;
 import org.apache.ignite.internal.cli.core.call.CallOutput;
@@ -81,20 +79,16 @@ public ConnectCall(
     @Override
     public CallOutput<String> execute(ConnectCallInput input) {
         String nodeUrl = input.url();
-        SessionInfo sessionInfo = session.info();
-        if (sessionInfo != null && Objects.equals(sessionInfo.nodeUrl(), nodeUrl)) {
-            // This username will be used for connect by the connection checker.
-            String username = input.username() != null
-                    ? input.username()
-                    : configManagerProvider.get().getCurrentProperty(BASIC_AUTHENTICATION_USERNAME.value());
-            if (Objects.equals(sessionInfo.username(), username)) {
+        try {
+            SessionInfo currentSessionInfo = session.info();
+            // Try without authentication first to check whether the authentication is enabled on the cluster.
+            SessionInfo sessionInfo = connectWithoutAuthentication(input);
+            // If we are already connected, check that the result of the connection attempt is the same.
+            if (currentSessionInfo != null && currentSessionInfo.equals(sessionInfo)) {
                 MessageUiComponent message = MessageUiComponent.fromMessage("You are already connected to %s", UiElements.url(nodeUrl));
                 return DefaultCallOutput.success(message.render());
             }
-        }
-        try {
-            // Try without authentication first to check whether the authentication is enabled on the cluster.
-            sessionInfo = connectWithoutAuthentication(input);
+
             if (sessionInfo == null) {
                 // Try with authentication
                 sessionInfo = connectionChecker.checkConnection(input);