This is an automated email from the ASF dual-hosted git repository. mpochatkin pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push: new 9769c06c2d IGNITE-22394 Check connection status in SQL CLI command (#4117) 9769c06c2d is described below commit 9769c06c2dd3dba41e4c3378e4e302f545cddc93 Author: Mikhail <pochat...@users.noreply.github.com> AuthorDate: Wed Aug 7 16:38:51 2024 +0300 IGNITE-22394 Check connection status in SQL CLI command (#4117) --- .../ItSqlReplCommandNotInitialedClusterTest.java | 93 ++++++++++++++++++++++ .../internal/cli/commands/sql/SqlCommand.java | 27 ++++++- .../internal/cli/commands/sql/SqlReplCommand.java | 26 +++++- .../cli/core/exception/ExceptionHandlers.java | 13 +-- .../handler/DefaultExceptionHandlers.java | 1 - .../exception/handler/SqlExceptionHandler.java | 28 ++++--- .../ignite/internal/cli/sql/SqlQueryResult.java | 6 +- .../internal/ClusterPerClassIntegrationTest.java | 6 +- 8 files changed, 169 insertions(+), 31 deletions(-) diff --git a/modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/sql/ItSqlReplCommandNotInitialedClusterTest.java b/modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/sql/ItSqlReplCommandNotInitialedClusterTest.java new file mode 100644 index 0000000000..f91675c0b0 --- /dev/null +++ b/modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/sql/ItSqlReplCommandNotInitialedClusterTest.java @@ -0,0 +1,93 @@ +/* + * 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.sql; + +import static org.junit.jupiter.api.Assertions.assertAll; + +import io.micronaut.context.annotation.Bean; +import io.micronaut.context.annotation.Replaces; +import org.apache.ignite.IgniteServer; +import org.apache.ignite.InitParameters; +import org.apache.ignite.internal.cli.CliIntegrationTest; +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 { + private final Session session = new Session(); + + @Bean + @Replaces(ReplExecutorProvider.class) + public ReplExecutorProvider replExecutorProvider() { + return () -> repl -> {}; + } + + @Bean + @Replaces(Session.class) + public Session session() { + return session; + } + + @Override + protected boolean needInitializeCluster() { + return false; + } + + @Test + @DisplayName("Should throw error because cluster not initialized.") + void nonExistingFile() { + execute("sql", "--jdbc-url", JDBC_URL, "CREATE TABLE T(K INT PRIMARY KEY)"); + + assertAll( + this::assertOutputIsEmpty, + () -> assertErrOutputContains("Connection refused:") + ); + + IgniteServer node0 = CLUSTER.startEmbeddedNode(0); + IgniteServer node1 = CLUSTER.startEmbeddedNode(1); + IgniteServer node2 = CLUSTER.startEmbeddedNode(2); + + session.onConnect(SessionInfo + .builder() + .jdbcUrl(JDBC_URL) + .nodeUrl(NODE_URL) + .build() + ); + + execute("sql", "--jdbc-url", JDBC_URL, "CREATE TABLE T(K INT PRIMARY KEY)"); + + assertAll( + this::assertOutputIsEmpty, + () -> assertErrOutputContains("Probably, you have not initialized the cluster, try to run") + ); + + node0.initCluster(InitParameters.builder().clusterName("cluster") + .cmgNodes(node0, node1, node2) + .metaStorageNodes(node0, node1, node2).build()); + + execute("sql", "--jdbc-url", JDBC_URL, "CREATE TABLE T(K INT PRIMARY KEY)"); + + assertAll( + this::assertOutputIsNotEmpty, + this::assertErrOutputIsEmpty + ); + } +} diff --git a/modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/sql/SqlCommand.java b/modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/sql/SqlCommand.java index 7e8ab191e4..4188a1fbc0 100644 --- a/modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/sql/SqlCommand.java +++ b/modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/sql/SqlCommand.java @@ -25,6 +25,7 @@ import static org.apache.ignite.internal.cli.commands.Options.Constants.PLAIN_OP import static org.apache.ignite.internal.cli.commands.Options.Constants.SCRIPT_FILE_OPTION; import static org.apache.ignite.internal.cli.commands.Options.Constants.SCRIPT_FILE_OPTION_DESC; +import jakarta.inject.Inject; import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -36,10 +37,16 @@ import org.apache.ignite.internal.cli.commands.BaseCommand; import org.apache.ignite.internal.cli.core.call.CallExecutionPipeline; import org.apache.ignite.internal.cli.core.call.StringCallInput; import org.apache.ignite.internal.cli.core.exception.ExceptionWriter; +import org.apache.ignite.internal.cli.core.exception.IgniteCliApiException; import org.apache.ignite.internal.cli.core.exception.IgniteCliException; +import org.apache.ignite.internal.cli.core.exception.handler.ClusterNotInitializedExceptionHandler; import org.apache.ignite.internal.cli.core.exception.handler.SqlExceptionHandler; +import org.apache.ignite.internal.cli.core.repl.Session; +import org.apache.ignite.internal.cli.core.rest.ApiClientFactory; import org.apache.ignite.internal.cli.decorators.SqlQueryResultDecorator; import org.apache.ignite.internal.cli.sql.SqlManager; +import org.apache.ignite.rest.client.api.ClusterManagementApi; +import org.apache.ignite.rest.client.invoker.ApiException; import picocli.CommandLine.ArgGroup; import picocli.CommandLine.Command; import picocli.CommandLine.Option; @@ -67,6 +74,12 @@ public class SqlCommand extends BaseCommand implements Callable<Integer> { private File file; } + @Inject + private Session session; + + @Inject + private ApiClientFactory clientFactory; + private static String extract(File file) { try { return String.join("\n", Files.readAllLines(file.toPath(), StandardCharsets.UTF_8)); @@ -90,7 +103,19 @@ public class SqlCommand extends BaseCommand implements Callable<Integer> { .verbose(verbose) .build().runPipeline(); } catch (SQLException e) { - return new SqlExceptionHandler().handle(ExceptionWriter.fromPrintWriter(spec.commandLine().getErr()), e); + String url = session.info() == null ? null : session.info().nodeUrl(); + + ExceptionWriter exceptionWriter = ExceptionWriter.fromPrintWriter(spec.commandLine().getErr()); + try { + if (url != null) { + new ClusterManagementApi(clientFactory.getClient(url)).clusterState(); + } + + return new SqlExceptionHandler().handle(exceptionWriter, e); + } catch (ApiException apiE) { + return new ClusterNotInitializedExceptionHandler("Failed to start sql repl mode", "cluster init") + .handle(exceptionWriter, new IgniteCliApiException(apiE, url)); + } } } } diff --git a/modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/sql/SqlReplCommand.java b/modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/sql/SqlReplCommand.java index d64cd8b73a..41718926cc 100644 --- a/modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/sql/SqlReplCommand.java +++ b/modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/sql/SqlReplCommand.java @@ -45,17 +45,23 @@ import org.apache.ignite.internal.cli.core.call.CallExecutionPipeline; import org.apache.ignite.internal.cli.core.call.StringCallInput; import org.apache.ignite.internal.cli.core.exception.ExceptionHandlers; import org.apache.ignite.internal.cli.core.exception.ExceptionWriter; +import org.apache.ignite.internal.cli.core.exception.IgniteCliApiException; import org.apache.ignite.internal.cli.core.exception.IgniteCliException; +import org.apache.ignite.internal.cli.core.exception.handler.ClusterNotInitializedExceptionHandler; import org.apache.ignite.internal.cli.core.exception.handler.SqlExceptionHandler; import org.apache.ignite.internal.cli.core.repl.EventListeningActivationPoint; import org.apache.ignite.internal.cli.core.repl.Repl; +import org.apache.ignite.internal.cli.core.repl.Session; import org.apache.ignite.internal.cli.core.repl.executor.RegistryCommandExecutor; import org.apache.ignite.internal.cli.core.repl.executor.ReplExecutorProvider; +import org.apache.ignite.internal.cli.core.rest.ApiClientFactory; import org.apache.ignite.internal.cli.core.style.AnsiStringSupport.Color; import org.apache.ignite.internal.cli.decorators.SqlQueryResultDecorator; import org.apache.ignite.internal.cli.sql.SqlManager; import org.apache.ignite.internal.cli.sql.SqlSchemaProvider; import org.apache.ignite.internal.util.StringUtils; +import org.apache.ignite.rest.client.api.ClusterManagementApi; +import org.apache.ignite.rest.client.invoker.ApiException; import org.jline.reader.EOFError; import org.jline.reader.Highlighter; import org.jline.reader.LineReader; @@ -102,6 +108,12 @@ public class SqlReplCommand extends BaseCommand implements Runnable { @Inject private ConfigManagerProvider configManagerProvider; + @Inject + private Session session; + + @Inject + private ApiClientFactory clientFactory; + private static String extract(File file) { try { return String.join("\n", Files.readAllLines(file.toPath(), StandardCharsets.UTF_8)); @@ -142,7 +154,19 @@ public class SqlReplCommand extends BaseCommand implements Runnable { } } } catch (SQLException e) { - new SqlExceptionHandler().handle(ExceptionWriter.fromPrintWriter(spec.commandLine().getErr()), e); + String url = session.info() == null ? null : session.info().nodeUrl(); + + ExceptionWriter exceptionWriter = ExceptionWriter.fromPrintWriter(spec.commandLine().getErr()); + try { + if (url != null) { + new ClusterManagementApi(clientFactory.getClient(url)).clusterState(); + } + + new SqlExceptionHandler().handle(exceptionWriter, e); + } catch (ApiException apiE) { + new ClusterNotInitializedExceptionHandler("Failed to start sql repl mode", "cluster init") + .handle(exceptionWriter, new IgniteCliApiException(apiE, url)); + } } } diff --git a/modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/ExceptionHandlers.java b/modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/ExceptionHandlers.java index 27b7ffcfef..1bb7a058f9 100644 --- a/modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/ExceptionHandlers.java +++ b/modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/ExceptionHandlers.java @@ -25,6 +25,7 @@ import java.util.Map; */ public class ExceptionHandlers { private final Map<Class<? extends Throwable>, ExceptionHandler<? extends Throwable>> map = new HashMap<>(); + private final ExceptionHandler<Throwable> defaultHandler; public ExceptionHandlers() { @@ -67,17 +68,6 @@ public class ExceptionHandlers { return processException(errOutput, e instanceof WrappedException ? e.getCause() : e); } - /** - * Handles an exception. - * - * @param e exception instance. - * @param <T> exception type. - * @return exit code. - */ - public <T extends Throwable> int handleException(T e) { - return processException(ExceptionWriter.nullWriter(), e instanceof WrappedException ? e.getCause() : e); - } - @SuppressWarnings("unchecked") private <T extends Throwable> int processException(ExceptionWriter errOutput, T e) { ExceptionHandler<T> exceptionHandler = (ExceptionHandler<T>) map.get(e.getClass()); @@ -87,5 +77,4 @@ public class ExceptionHandlers { return defaultHandler.handle(errOutput, e); } } - } diff --git a/modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/handler/DefaultExceptionHandlers.java b/modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/handler/DefaultExceptionHandlers.java index 4275455d02..a0c20eeb7c 100644 --- a/modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/handler/DefaultExceptionHandlers.java +++ b/modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/handler/DefaultExceptionHandlers.java @@ -29,7 +29,6 @@ public final class DefaultExceptionHandlers extends ExceptionHandlers { */ public DefaultExceptionHandlers() { addExceptionHandler(new FlowInterruptExceptionHandler()); - addExceptionHandler(new SqlExceptionHandler()); addExceptionHandler(new TimeoutExceptionHandler()); addExceptionHandler(new IgniteCliExceptionHandler()); addExceptionHandler(new IgniteCliApiExceptionHandler()); diff --git a/modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/handler/SqlExceptionHandler.java b/modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/handler/SqlExceptionHandler.java index 6773dab027..a3cba0b656 100644 --- a/modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/handler/SqlExceptionHandler.java +++ b/modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/handler/SqlExceptionHandler.java @@ -17,6 +17,8 @@ package org.apache.ignite.internal.cli.core.exception.handler; +import static org.apache.ignite.lang.ErrorGroup.extractCauseMessage; + import java.sql.SQLException; import java.util.HashMap; import java.util.Map; @@ -31,7 +33,6 @@ import org.apache.ignite.internal.jdbc.proto.SqlStateCode; import org.apache.ignite.internal.logger.IgniteLogger; import org.apache.ignite.internal.logger.Loggers; import org.apache.ignite.internal.util.ExceptionUtils; -import org.apache.ignite.lang.ErrorGroup; import org.apache.ignite.lang.ErrorGroups.Client; import org.apache.ignite.lang.ErrorGroups.Sql; import org.apache.ignite.lang.IgniteCheckedException; @@ -50,25 +51,26 @@ public class SqlExceptionHandler implements ExceptionHandler<SQLException> { public static final String CLIENT_CONNECTION_FAILED_MESSAGE = "Connection failed"; public static final String CONNECTION_BROKE_MESSAGE = "Connection error"; + public static final String UNRECOGNIZED_ERROR_MESSAGE = "Unrecognized error while processing SQL query "; private final Map<Integer, Function<IgniteException, ErrorComponentBuilder>> sqlExceptionMappers = new HashMap<>(); /** Default constructor. */ public SqlExceptionHandler() { - sqlExceptionMappers.put(Client.CONNECTION_ERR, this::connectionErrUiComponent); - sqlExceptionMappers.put(Sql.STMT_PARSE_ERR, this::sqlParseErrUiComponent); + sqlExceptionMappers.put(Client.CONNECTION_ERR, SqlExceptionHandler::connectionErrUiComponent); + sqlExceptionMappers.put(Sql.STMT_PARSE_ERR, SqlExceptionHandler::sqlParseErrUiComponent); } - private ErrorComponentBuilder sqlParseErrUiComponent(IgniteException e) { + private static ErrorComponentBuilder sqlParseErrUiComponent(IgniteException e) { return fromExWithHeader(PARSING_ERROR_MESSAGE, e.code(), e.traceId(), e.getMessage()); } - private ErrorComponentBuilder unrecognizedErrComponent(IgniteException e) { + private static ErrorComponentBuilder unrecognizedErrComponent(IgniteException e) { return fromExWithHeader(UNRECOGNIZED_ERROR_MESSAGE, e.code(), e.traceId(), e.getMessage()); } - private ErrorComponentBuilder connectionErrUiComponent(IgniteException e) { + private static ErrorComponentBuilder connectionErrUiComponent(IgniteException e) { if (e.getCause() instanceof IgniteClientConnectionException) { IgniteClientConnectionException cause = (IgniteClientConnectionException) e.getCause(); return fromExWithHeader(CLIENT_CONNECTION_FAILED_MESSAGE, cause.code(), cause.traceId(), cause.getMessage()); @@ -82,7 +84,7 @@ public class SqlExceptionHandler implements ExceptionHandler<SQLException> { .header(header) .errorCode(String.valueOf(errorCode)) .traceId(traceId) - .details(ErrorGroup.extractCauseMessage(message)); + .details(extractCauseMessage(message)); } @Override @@ -102,16 +104,16 @@ public class SqlExceptionHandler implements ExceptionHandler<SQLException> { case SqlStateCode.CONNECTION_FAILURE: case SqlStateCode.CONNECTION_CLOSED: case SqlStateCode.CONNECTION_REJECTED: - errorComponentBuilder.header(CONNECTION_BROKE_MESSAGE).verbose(ErrorGroup.extractCauseMessage(e.getMessage())); + errorComponentBuilder.header(CONNECTION_BROKE_MESSAGE).verbose(extractCauseMessage(e.getMessage())); break; case SqlStateCode.PARSING_EXCEPTION: - errorComponentBuilder.header(PARSING_ERROR_MESSAGE).details(ErrorGroup.extractCauseMessage(e.getMessage())); + errorComponentBuilder.header(PARSING_ERROR_MESSAGE).details(extractCauseMessage(e.getMessage())); break; case SqlStateCode.INVALID_PARAMETER_VALUE: - errorComponentBuilder.header(INVALID_PARAMETER_MESSAGE).verbose(ErrorGroup.extractCauseMessage(e.getMessage())); + errorComponentBuilder.header(INVALID_PARAMETER_MESSAGE).verbose(extractCauseMessage(e.getMessage())); break; case SqlStateCode.CLIENT_CONNECTION_FAILED: - errorComponentBuilder.header(CLIENT_CONNECTION_FAILED_MESSAGE).verbose(ErrorGroup.extractCauseMessage(e.getMessage())); + errorComponentBuilder.header(CLIENT_CONNECTION_FAILED_MESSAGE).verbose(extractCauseMessage(e.getMessage())); break; default: LOG.error("Unrecognized error", e); @@ -124,7 +126,7 @@ public class SqlExceptionHandler implements ExceptionHandler<SQLException> { /** Handles IgniteException that has more information like error code and trace id. */ private int handleIgniteException(ExceptionWriter err, IgniteException e) { - var errorComponentBuilder = sqlExceptionMappers.getOrDefault(e.code(), this::unrecognizedErrComponent); + var errorComponentBuilder = sqlExceptionMappers.getOrDefault(e.code(), SqlExceptionHandler::unrecognizedErrComponent); String renderedError = errorComponentBuilder.apply(e).build().render(); err.write(renderedError); @@ -132,7 +134,7 @@ public class SqlExceptionHandler implements ExceptionHandler<SQLException> { return 1; } - private int handleIgniteCheckedException(ExceptionWriter err, IgniteCheckedException e) { + private static int handleIgniteCheckedException(ExceptionWriter err, IgniteCheckedException e) { String renderedError = fromExWithHeader(UNRECOGNIZED_ERROR_MESSAGE, e.code(), e.traceId(), e.getMessage()) .build().render(); err.write(renderedError); diff --git a/modules/cli/src/main/java/org/apache/ignite/internal/cli/sql/SqlQueryResult.java b/modules/cli/src/main/java/org/apache/ignite/internal/cli/sql/SqlQueryResult.java index fd85e85605..0972db073c 100644 --- a/modules/cli/src/main/java/org/apache/ignite/internal/cli/sql/SqlQueryResult.java +++ b/modules/cli/src/main/java/org/apache/ignite/internal/cli/sql/SqlQueryResult.java @@ -21,11 +21,13 @@ import org.apache.ignite.internal.cli.core.decorator.Decorator; import org.apache.ignite.internal.cli.core.decorator.TerminalOutput; import org.apache.ignite.internal.cli.decorators.TableDecorator; import org.apache.ignite.internal.cli.sql.table.Table; +import org.jetbrains.annotations.Nullable; /** * Composite object of sql query result. */ public class SqlQueryResult { + @Nullable private final Table<String> table; private final String message; @@ -35,7 +37,7 @@ public class SqlQueryResult { * * @param table non null result table. */ - public SqlQueryResult(Table<String> table) { + public SqlQueryResult(@Nullable Table<String> table) { this(table, null); } @@ -48,7 +50,7 @@ public class SqlQueryResult { this(null, message); } - private SqlQueryResult(Table<String> table, String message) { + private SqlQueryResult(@Nullable Table<String> table, String message) { this.table = table; this.message = message; } diff --git a/modules/runner/src/testFixtures/java/org/apache/ignite/internal/ClusterPerClassIntegrationTest.java b/modules/runner/src/testFixtures/java/org/apache/ignite/internal/ClusterPerClassIntegrationTest.java index 371def0ff2..999f36727b 100644 --- a/modules/runner/src/testFixtures/java/org/apache/ignite/internal/ClusterPerClassIntegrationTest.java +++ b/modules/runner/src/testFixtures/java/org/apache/ignite/internal/ClusterPerClassIntegrationTest.java @@ -103,7 +103,7 @@ public abstract class ClusterPerClassIntegrationTest extends BaseIgniteAbstractT protected void beforeAll(TestInfo testInfo) { CLUSTER = new Cluster(testInfo, WORK_DIR, getNodeBootstrapConfigTemplate()); - if (initialNodes() > 0) { + if (initialNodes() > 0 && needInitializeCluster()) { CLUSTER.startAndInit(initialNodes(), cmgMetastoreNodes(), this::configureInitParameters); } } @@ -121,6 +121,10 @@ public abstract class ClusterPerClassIntegrationTest extends BaseIgniteAbstractT return new int[] { 0 }; } + protected boolean needInitializeCluster() { + return true; + } + /** * This method can be overridden to add custom init parameters during cluster initialization. */