You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/05/19 16:23:26 UTC
[GitHub] [ignite-3] PakhomovAlexander opened a new pull request, #816: IGNITE-16943: Implement Micronaut REST
PakhomovAlexander opened a new pull request, #816:
URL: https://github.com/apache/ignite-3/pull/816
- Introduce micronaut-based rest server
- Generate openapi.yaml file from server implementation
- Add rest-client module that produces auto-generated rest client
- Use micronaut-serde library in order to avoid jackson dependency
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander closed pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander closed pull request #816: IGNITE-16943: Implement Micronaut REST
URL: https://github.com/apache/ignite-3/pull/816
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r887337627
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java:
##########
@@ -17,151 +17,147 @@
package org.apache.ignite.internal.rest;
-import io.netty.bootstrap.ServerBootstrap;
-import io.netty.channel.Channel;
-import io.netty.channel.ChannelFuture;
-import io.netty.handler.logging.LogLevel;
-import io.netty.handler.logging.LoggingHandler;
+import io.micronaut.context.ApplicationContext;
+import io.micronaut.http.server.exceptions.ServerStartupException;
+import io.micronaut.openapi.annotation.OpenAPIInclude;
+import io.micronaut.runtime.Micronaut;
+import io.micronaut.runtime.exceptions.ApplicationStartupException;
+import io.swagger.v3.oas.annotations.OpenAPIDefinition;
+import io.swagger.v3.oas.annotations.info.Contact;
+import io.swagger.v3.oas.annotations.info.Info;
+import io.swagger.v3.oas.annotations.info.License;
import java.net.BindException;
-import java.net.InetSocketAddress;
-import java.util.function.Consumer;
+import java.util.List;
+import java.util.Map;
import org.apache.ignite.configuration.schemas.rest.RestConfiguration;
import org.apache.ignite.configuration.schemas.rest.RestView;
-import org.apache.ignite.internal.configuration.ConfigurationManager;
-import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController;
import org.apache.ignite.internal.manager.IgniteComponent;
-import org.apache.ignite.internal.rest.api.RestHandlersRegister;
-import org.apache.ignite.internal.rest.api.Routes;
-import org.apache.ignite.internal.rest.netty.RestApiInitializer;
-import org.apache.ignite.internal.rest.routes.Router;
-import org.apache.ignite.internal.rest.routes.SimpleRouter;
-import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.internal.rest.api.RestFactory;
+import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController;
+import org.apache.ignite.internal.rest.configuration.NodeConfigurationController;
import org.apache.ignite.lang.IgniteInternalException;
import org.apache.ignite.lang.IgniteLogger;
-import org.apache.ignite.network.NettyBootstrapFactory;
/**
- * Rest component is responsible for starting REST endpoints.
+ * Rest module is responsible for starting a REST endpoints for accessing and managing configuration.
*
- * <p>It is started on port 10300 by default, but it is possible to change this in configuration itself. Refer to default config file in
+ * <p>It is started on port 10300 by default but it is possible to change this in configuration itself. Refer to default config file in
* resources for the example.
*/
-public class RestComponent implements RestHandlersRegister, IgniteComponent {
+@OpenAPIDefinition(info = @Info(title = "Ignite REST module", version = "3.0.0-alpha", license = @License(name = "Apache 2.0", url = "https://ignite.apache.org"), contact = @Contact(email = "user@ignite.apache.org")))
+@OpenAPIInclude(classes = {ClusterConfigurationController.class, NodeConfigurationController.class, ClusterManagementController.class})
+public class RestComponent implements IgniteComponent {
+ /** Default port. */
+ public static final int DFLT_PORT = 10300;
+
/** Ignite logger. */
private final IgniteLogger log = IgniteLogger.forClass(RestComponent.class);
- /** Node configuration register. */
- private final ConfigurationRegistry nodeCfgRegistry;
-
- /** Netty bootstrap factory. */
- private final NettyBootstrapFactory bootstrapFactory;
-
- private final SimpleRouter router = new SimpleRouter();
-
- /** Netty channel. */
- private volatile Channel channel;
+ /** Factories that produce beans needed for REST controllers. */
+ private final List<RestFactory> restFactories;
+ private final RestConfiguration restConfiguration;
+ /** Server host. */
+ private final String host = "localhost";
+ /** Micronaut application context. */
+ private ApplicationContext context;
+ /** Server port. */
+ private int port;
/**
* Creates a new instance of REST module.
- *
- * @param nodeCfgMgr Node configuration manager.
- * @param bootstrapFactory Netty bootstrap factory.
*/
- public RestComponent(ConfigurationManager nodeCfgMgr, NettyBootstrapFactory bootstrapFactory) {
- nodeCfgRegistry = nodeCfgMgr.configurationRegistry();
-
- this.bootstrapFactory = bootstrapFactory;
- }
-
- /** {@inheritDoc} */
- @Override
- public void registerHandlers(Consumer<Routes> registerAction) {
- registerAction.accept(router);
+ public RestComponent(List<RestFactory> restFactories, RestConfiguration restConfiguration) {
+ this.restFactories = restFactories;
+ this.restConfiguration = restConfiguration;
}
/** {@inheritDoc} */
@Override
public void start() {
- if (channel != null) {
- throw new IgniteException("RestModule is already started.");
- }
-
- channel = startRestEndpoint(router).channel();
- }
-
- /**
- * Start endpoint.
- *
- * @param router Dispatcher of http requests.
- * @return Future which will be notified when this channel is closed.
- * @throws RuntimeException if this module cannot be bound to a port.
- */
- private ChannelFuture startRestEndpoint(Router router) {
- RestView restConfigurationView = nodeCfgRegistry.getConfiguration(RestConfiguration.KEY).value();
+ RestView restConfigurationView = restConfiguration.value();
int desiredPort = restConfigurationView.port();
int portRange = restConfigurationView.portRange();
- int port = 0;
- Channel ch = null;
-
- ServerBootstrap bootstrap = bootstrapFactory.createServerBootstrap()
- .handler(new LoggingHandler(LogLevel.INFO))
- .childHandler(new RestApiInitializer(router));
-
for (int portCandidate = desiredPort; portCandidate <= desiredPort + portRange; portCandidate++) {
- ChannelFuture bindRes = bootstrap.bind(portCandidate).awaitUninterruptibly();
-
- if (bindRes.isSuccess()) {
- ch = bindRes.channel();
+ try {
+ context = buildMicronautContext(portCandidate).start();
port = portCandidate;
+ log.info("REST protocol started successfully");
break;
- } else if (!(bindRes.cause() instanceof BindException)) {
- throw new RuntimeException(bindRes.cause());
+ } catch (ApplicationStartupException e) {
+ log.error("Got exception " + e.getCause() + " during node start on port " + portCandidate + " , trying again");
}
}
- if (ch == null) {
- String msg = "Cannot start REST endpoint. "
- + "All ports in range [" + desiredPort + ", " + (desiredPort + portRange) + "] are in use.";
+ if (context == null) {
+ String msg = "Cannot start REST endpoint. " + "All ports in range [" + desiredPort + ", " + (desiredPort + portRange)
+ + "] are in use.";
log.error(msg);
throw new RuntimeException(msg);
}
+ }
+
+ private Micronaut buildMicronautContext(int portCandidate) {
+ Micronaut micronaut = Micronaut.build("");
+ setFactories(micronaut);
+ return micronaut
+ .properties(Map.of("micronaut.server.port", portCandidate))
+ .banner(false)
+ .mapError(ServerStartupException.class, this::mapServerStartupException)
+ .mapError(ApplicationStartupException.class, ex -> -1);
+ }
- if (log.isInfoEnabled()) {
- log.info("REST protocol started successfully on port " + port);
+ private void setFactories(Micronaut micronaut) {
+ for (var factory : restFactories) {
+ micronaut.singletons(factory);
}
+ }
- return ch.closeFuture();
+ private int mapServerStartupException(ServerStartupException exception) {
+ if (exception.getCause() instanceof BindException) {
+ return -1;
Review Comment:
Added a comment to the code.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r887790764
##########
modules/configuration/src/main/java/org/apache/ignite/internal/rest/configuration/exception/handler/IgniteExceptionHandler.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * 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.rest.configuration.exception.handler;
+
+import io.micronaut.context.annotation.Requires;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpResponse;
+import io.micronaut.http.annotation.Produces;
+import io.micronaut.http.server.exceptions.ExceptionHandler;
+import jakarta.inject.Singleton;
+import org.apache.ignite.configuration.validation.ConfigurationValidationException;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.apache.ignite.lang.IgniteException;
+
+/**
+ * Handles {@link ConfigurationValidationException} and represents it as a rest response.
+ */
+@Produces
+@Singleton
+@Requires(classes = {IgniteException.class, ExceptionHandler.class})
+public class IgniteExceptionHandler implements ExceptionHandler<IgniteException, HttpResponse<ErrorResult>> {
+
+ @Override
+ public HttpResponse<ErrorResult> handle(HttpRequest request, IgniteException exception) {
+ ErrorResult errorResult = new ErrorResult("APPLICATION_EXCEPTION", exception.getMessage());
+ return HttpResponse.badRequest().body(errorResult);
Review Comment:
Same here, I don't think it should be a "bad request"
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalExceptionHandler.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * 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.cluster.management.rest.exception.handler;
+
+import io.micronaut.context.annotation.Requires;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpResponse;
+import io.micronaut.http.annotation.Produces;
+import io.micronaut.http.server.exceptions.ExceptionHandler;
+import jakarta.inject.Singleton;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.apache.ignite.lang.IgniteInternalException;
+
+/**
+ * Handles {@link IgniteInternalException} and represents it as a rest response.
+ */
+@Produces
+@Singleton
+@Requires(classes = {IgniteInternalException.class, ExceptionHandler.class})
+public class IgniteInternalExceptionHandler implements ExceptionHandler<IgniteInternalException, HttpResponse<ErrorResult>> {
+
+ @Override
+ public HttpResponse<ErrorResult> handle(HttpRequest request, IgniteInternalException exception) {
+ ErrorResult errorResult = new ErrorResult("INITIALIZATION_EXCEPTION", exception.getMessage());
+ return HttpResponse.badRequest().body(errorResult);
Review Comment:
Are you sure it should be a "bad request` errror?
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java:
##########
@@ -17,151 +17,161 @@
package org.apache.ignite.internal.rest;
-import io.netty.bootstrap.ServerBootstrap;
-import io.netty.channel.Channel;
-import io.netty.channel.ChannelFuture;
-import io.netty.handler.logging.LogLevel;
-import io.netty.handler.logging.LoggingHandler;
+import io.micronaut.context.ApplicationContext;
+import io.micronaut.http.server.exceptions.ServerStartupException;
+import io.micronaut.openapi.annotation.OpenAPIInclude;
+import io.micronaut.runtime.Micronaut;
+import io.micronaut.runtime.exceptions.ApplicationStartupException;
+import io.swagger.v3.oas.annotations.OpenAPIDefinition;
+import io.swagger.v3.oas.annotations.info.Contact;
+import io.swagger.v3.oas.annotations.info.Info;
+import io.swagger.v3.oas.annotations.info.License;
import java.net.BindException;
-import java.net.InetSocketAddress;
-import java.util.function.Consumer;
+import java.util.List;
+import java.util.Map;
import org.apache.ignite.configuration.schemas.rest.RestConfiguration;
import org.apache.ignite.configuration.schemas.rest.RestView;
-import org.apache.ignite.internal.configuration.ConfigurationManager;
-import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController;
import org.apache.ignite.internal.manager.IgniteComponent;
-import org.apache.ignite.internal.rest.api.RestHandlersRegister;
-import org.apache.ignite.internal.rest.api.Routes;
-import org.apache.ignite.internal.rest.netty.RestApiInitializer;
-import org.apache.ignite.internal.rest.routes.Router;
-import org.apache.ignite.internal.rest.routes.SimpleRouter;
-import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.internal.rest.api.RestFactory;
+import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController;
+import org.apache.ignite.internal.rest.configuration.NodeConfigurationController;
import org.apache.ignite.lang.IgniteInternalException;
import org.apache.ignite.lang.IgniteLogger;
-import org.apache.ignite.network.NettyBootstrapFactory;
/**
- * Rest component is responsible for starting REST endpoints.
+ * Rest module is responsible for starting REST endpoints for accessing and managing configuration.
*
- * <p>It is started on port 10300 by default, but it is possible to change this in configuration itself. Refer to default config file in
+ * <p>It is started on port 10300 by default but it is possible to change this in configuration itself. Refer to default config file in
* resources for the example.
*/
-public class RestComponent implements RestHandlersRegister, IgniteComponent {
+@OpenAPIDefinition(info = @Info(title = "Ignite REST module", version = "3.0.0-alpha", license = @License(name = "Apache 2.0", url = "https://ignite.apache.org"), contact = @Contact(email = "user@ignite.apache.org")))
+@OpenAPIInclude(classes = {ClusterConfigurationController.class, NodeConfigurationController.class, ClusterManagementController.class})
+public class RestComponent implements IgniteComponent {
+ /** Default port. */
+ public static final int DFLT_PORT = 10300;
+
+ /** Server host. */
+ private static final String LOCALHOST = "localhost";
+
/** Ignite logger. */
private final IgniteLogger log = IgniteLogger.forClass(RestComponent.class);
- /** Node configuration register. */
- private final ConfigurationRegistry nodeCfgRegistry;
+ /** Factories that produce beans needed for REST controllers. */
+ private final List<RestFactory> restFactories;
- /** Netty bootstrap factory. */
- private final NettyBootstrapFactory bootstrapFactory;
+ private final RestConfiguration restConfiguration;
- private final SimpleRouter router = new SimpleRouter();
+ /** Micronaut application context. */
+ private volatile ApplicationContext context;
- /** Netty channel. */
- private volatile Channel channel;
+ /** Server port. */
+ private int port;
/**
* Creates a new instance of REST module.
- *
- * @param nodeCfgMgr Node configuration manager.
- * @param bootstrapFactory Netty bootstrap factory.
*/
- public RestComponent(ConfigurationManager nodeCfgMgr, NettyBootstrapFactory bootstrapFactory) {
- nodeCfgRegistry = nodeCfgMgr.configurationRegistry();
-
- this.bootstrapFactory = bootstrapFactory;
- }
-
- /** {@inheritDoc} */
- @Override
- public void registerHandlers(Consumer<Routes> registerAction) {
- registerAction.accept(router);
+ public RestComponent(List<RestFactory> restFactories, RestConfiguration restConfiguration) {
+ this.restFactories = restFactories;
+ this.restConfiguration = restConfiguration;
}
/** {@inheritDoc} */
@Override
public void start() {
- if (channel != null) {
- throw new IgniteException("RestModule is already started.");
- }
-
- channel = startRestEndpoint(router).channel();
- }
-
- /**
- * Start endpoint.
- *
- * @param router Dispatcher of http requests.
- * @return Future which will be notified when this channel is closed.
- * @throws RuntimeException if this module cannot be bound to a port.
- */
- private ChannelFuture startRestEndpoint(Router router) {
- RestView restConfigurationView = nodeCfgRegistry.getConfiguration(RestConfiguration.KEY).value();
+ RestView restConfigurationView = restConfiguration.value();
int desiredPort = restConfigurationView.port();
int portRange = restConfigurationView.portRange();
- int port = 0;
- Channel ch = null;
-
- ServerBootstrap bootstrap = bootstrapFactory.createServerBootstrap()
- .handler(new LoggingHandler(LogLevel.INFO))
- .childHandler(new RestApiInitializer(router));
-
for (int portCandidate = desiredPort; portCandidate <= desiredPort + portRange; portCandidate++) {
- ChannelFuture bindRes = bootstrap.bind(portCandidate).awaitUninterruptibly();
-
- if (bindRes.isSuccess()) {
- ch = bindRes.channel();
+ try {
+ context = buildMicronautContext(portCandidate).start();
port = portCandidate;
+ log.info("REST protocol started successfully");
+ return;
+ } catch (ApplicationStartupException e) {
+ if (isBindExceptionInside(e)) {
Review Comment:
I would suggest to introduce a `findBindException` method that will return an exception or `null`. `e.getCause().getCause().getCause()` looks really weird
##########
parent/pom.xml:
##########
@@ -473,6 +475,55 @@
<version>${micronaut.version}</version>
</dependency>
+ <dependency>
+ <groupId>io.micronaut</groupId>
Review Comment:
So, what about using a `bom`?
##########
modules/cli/src/integrationTest/java/org/apache/ignite/cli/ItConfigCommandTest.java:
##########
@@ -169,11 +166,9 @@ public void partialGet() {
assertEquals(0, exitCode);
- JSONObject outResult = (JSONObject) JSONValue.parse(removeTrailingQuotes(unescapeQuotes(out.toString(UTF_8))));
-
- assertTrue(outResult.containsKey("inbound"));
+ assertTrue(out.toString(UTF_8).contains("\"inbound\""));
Review Comment:
It's a bit strange that you decided to use `assertThat` above, but not here)
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java:
##########
@@ -17,151 +17,161 @@
package org.apache.ignite.internal.rest;
-import io.netty.bootstrap.ServerBootstrap;
-import io.netty.channel.Channel;
-import io.netty.channel.ChannelFuture;
-import io.netty.handler.logging.LogLevel;
-import io.netty.handler.logging.LoggingHandler;
+import io.micronaut.context.ApplicationContext;
+import io.micronaut.http.server.exceptions.ServerStartupException;
+import io.micronaut.openapi.annotation.OpenAPIInclude;
+import io.micronaut.runtime.Micronaut;
+import io.micronaut.runtime.exceptions.ApplicationStartupException;
+import io.swagger.v3.oas.annotations.OpenAPIDefinition;
+import io.swagger.v3.oas.annotations.info.Contact;
+import io.swagger.v3.oas.annotations.info.Info;
+import io.swagger.v3.oas.annotations.info.License;
import java.net.BindException;
-import java.net.InetSocketAddress;
-import java.util.function.Consumer;
+import java.util.List;
+import java.util.Map;
import org.apache.ignite.configuration.schemas.rest.RestConfiguration;
import org.apache.ignite.configuration.schemas.rest.RestView;
-import org.apache.ignite.internal.configuration.ConfigurationManager;
-import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController;
import org.apache.ignite.internal.manager.IgniteComponent;
-import org.apache.ignite.internal.rest.api.RestHandlersRegister;
-import org.apache.ignite.internal.rest.api.Routes;
-import org.apache.ignite.internal.rest.netty.RestApiInitializer;
-import org.apache.ignite.internal.rest.routes.Router;
-import org.apache.ignite.internal.rest.routes.SimpleRouter;
-import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.internal.rest.api.RestFactory;
+import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController;
+import org.apache.ignite.internal.rest.configuration.NodeConfigurationController;
import org.apache.ignite.lang.IgniteInternalException;
import org.apache.ignite.lang.IgniteLogger;
-import org.apache.ignite.network.NettyBootstrapFactory;
/**
- * Rest component is responsible for starting REST endpoints.
+ * Rest module is responsible for starting REST endpoints for accessing and managing configuration.
*
- * <p>It is started on port 10300 by default, but it is possible to change this in configuration itself. Refer to default config file in
+ * <p>It is started on port 10300 by default but it is possible to change this in configuration itself. Refer to default config file in
* resources for the example.
*/
-public class RestComponent implements RestHandlersRegister, IgniteComponent {
+@OpenAPIDefinition(info = @Info(title = "Ignite REST module", version = "3.0.0-alpha", license = @License(name = "Apache 2.0", url = "https://ignite.apache.org"), contact = @Contact(email = "user@ignite.apache.org")))
+@OpenAPIInclude(classes = {ClusterConfigurationController.class, NodeConfigurationController.class, ClusterManagementController.class})
+public class RestComponent implements IgniteComponent {
+ /** Default port. */
+ public static final int DFLT_PORT = 10300;
+
+ /** Server host. */
+ private static final String LOCALHOST = "localhost";
+
/** Ignite logger. */
private final IgniteLogger log = IgniteLogger.forClass(RestComponent.class);
- /** Node configuration register. */
- private final ConfigurationRegistry nodeCfgRegistry;
+ /** Factories that produce beans needed for REST controllers. */
+ private final List<RestFactory> restFactories;
- /** Netty bootstrap factory. */
- private final NettyBootstrapFactory bootstrapFactory;
+ private final RestConfiguration restConfiguration;
- private final SimpleRouter router = new SimpleRouter();
+ /** Micronaut application context. */
+ private volatile ApplicationContext context;
- /** Netty channel. */
- private volatile Channel channel;
+ /** Server port. */
+ private int port;
/**
* Creates a new instance of REST module.
- *
- * @param nodeCfgMgr Node configuration manager.
- * @param bootstrapFactory Netty bootstrap factory.
*/
- public RestComponent(ConfigurationManager nodeCfgMgr, NettyBootstrapFactory bootstrapFactory) {
- nodeCfgRegistry = nodeCfgMgr.configurationRegistry();
-
- this.bootstrapFactory = bootstrapFactory;
- }
-
- /** {@inheritDoc} */
- @Override
- public void registerHandlers(Consumer<Routes> registerAction) {
- registerAction.accept(router);
+ public RestComponent(List<RestFactory> restFactories, RestConfiguration restConfiguration) {
+ this.restFactories = restFactories;
+ this.restConfiguration = restConfiguration;
}
/** {@inheritDoc} */
@Override
public void start() {
- if (channel != null) {
- throw new IgniteException("RestModule is already started.");
- }
-
- channel = startRestEndpoint(router).channel();
- }
-
- /**
- * Start endpoint.
- *
- * @param router Dispatcher of http requests.
- * @return Future which will be notified when this channel is closed.
- * @throws RuntimeException if this module cannot be bound to a port.
- */
- private ChannelFuture startRestEndpoint(Router router) {
- RestView restConfigurationView = nodeCfgRegistry.getConfiguration(RestConfiguration.KEY).value();
+ RestView restConfigurationView = restConfiguration.value();
int desiredPort = restConfigurationView.port();
int portRange = restConfigurationView.portRange();
- int port = 0;
- Channel ch = null;
-
- ServerBootstrap bootstrap = bootstrapFactory.createServerBootstrap()
- .handler(new LoggingHandler(LogLevel.INFO))
- .childHandler(new RestApiInitializer(router));
-
for (int portCandidate = desiredPort; portCandidate <= desiredPort + portRange; portCandidate++) {
- ChannelFuture bindRes = bootstrap.bind(portCandidate).awaitUninterruptibly();
-
- if (bindRes.isSuccess()) {
- ch = bindRes.channel();
+ try {
+ context = buildMicronautContext(portCandidate).start();
port = portCandidate;
Review Comment:
unfortunately this is not correct. If you want to avoid making `port` variable `volatile` and guard it with `context` accesses, you must always write to the `port` variable **before** writing to the `context` variable. Same applies to reading, but in reverse order
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java:
##########
@@ -17,151 +17,161 @@
package org.apache.ignite.internal.rest;
-import io.netty.bootstrap.ServerBootstrap;
-import io.netty.channel.Channel;
-import io.netty.channel.ChannelFuture;
-import io.netty.handler.logging.LogLevel;
-import io.netty.handler.logging.LoggingHandler;
+import io.micronaut.context.ApplicationContext;
+import io.micronaut.http.server.exceptions.ServerStartupException;
+import io.micronaut.openapi.annotation.OpenAPIInclude;
+import io.micronaut.runtime.Micronaut;
+import io.micronaut.runtime.exceptions.ApplicationStartupException;
+import io.swagger.v3.oas.annotations.OpenAPIDefinition;
+import io.swagger.v3.oas.annotations.info.Contact;
+import io.swagger.v3.oas.annotations.info.Info;
+import io.swagger.v3.oas.annotations.info.License;
import java.net.BindException;
-import java.net.InetSocketAddress;
-import java.util.function.Consumer;
+import java.util.List;
+import java.util.Map;
import org.apache.ignite.configuration.schemas.rest.RestConfiguration;
import org.apache.ignite.configuration.schemas.rest.RestView;
-import org.apache.ignite.internal.configuration.ConfigurationManager;
-import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController;
import org.apache.ignite.internal.manager.IgniteComponent;
-import org.apache.ignite.internal.rest.api.RestHandlersRegister;
-import org.apache.ignite.internal.rest.api.Routes;
-import org.apache.ignite.internal.rest.netty.RestApiInitializer;
-import org.apache.ignite.internal.rest.routes.Router;
-import org.apache.ignite.internal.rest.routes.SimpleRouter;
-import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.internal.rest.api.RestFactory;
+import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController;
+import org.apache.ignite.internal.rest.configuration.NodeConfigurationController;
import org.apache.ignite.lang.IgniteInternalException;
import org.apache.ignite.lang.IgniteLogger;
-import org.apache.ignite.network.NettyBootstrapFactory;
/**
- * Rest component is responsible for starting REST endpoints.
+ * Rest module is responsible for starting REST endpoints for accessing and managing configuration.
*
- * <p>It is started on port 10300 by default, but it is possible to change this in configuration itself. Refer to default config file in
+ * <p>It is started on port 10300 by default but it is possible to change this in configuration itself. Refer to default config file in
* resources for the example.
*/
-public class RestComponent implements RestHandlersRegister, IgniteComponent {
+@OpenAPIDefinition(info = @Info(title = "Ignite REST module", version = "3.0.0-alpha", license = @License(name = "Apache 2.0", url = "https://ignite.apache.org"), contact = @Contact(email = "user@ignite.apache.org")))
+@OpenAPIInclude(classes = {ClusterConfigurationController.class, NodeConfigurationController.class, ClusterManagementController.class})
+public class RestComponent implements IgniteComponent {
+ /** Default port. */
+ public static final int DFLT_PORT = 10300;
+
+ /** Server host. */
+ private static final String LOCALHOST = "localhost";
+
/** Ignite logger. */
private final IgniteLogger log = IgniteLogger.forClass(RestComponent.class);
- /** Node configuration register. */
- private final ConfigurationRegistry nodeCfgRegistry;
+ /** Factories that produce beans needed for REST controllers. */
+ private final List<RestFactory> restFactories;
- /** Netty bootstrap factory. */
- private final NettyBootstrapFactory bootstrapFactory;
+ private final RestConfiguration restConfiguration;
- private final SimpleRouter router = new SimpleRouter();
+ /** Micronaut application context. */
+ private volatile ApplicationContext context;
- /** Netty channel. */
- private volatile Channel channel;
+ /** Server port. */
+ private int port;
/**
* Creates a new instance of REST module.
- *
- * @param nodeCfgMgr Node configuration manager.
- * @param bootstrapFactory Netty bootstrap factory.
*/
- public RestComponent(ConfigurationManager nodeCfgMgr, NettyBootstrapFactory bootstrapFactory) {
- nodeCfgRegistry = nodeCfgMgr.configurationRegistry();
-
- this.bootstrapFactory = bootstrapFactory;
- }
-
- /** {@inheritDoc} */
- @Override
- public void registerHandlers(Consumer<Routes> registerAction) {
- registerAction.accept(router);
+ public RestComponent(List<RestFactory> restFactories, RestConfiguration restConfiguration) {
+ this.restFactories = restFactories;
+ this.restConfiguration = restConfiguration;
}
/** {@inheritDoc} */
@Override
public void start() {
- if (channel != null) {
- throw new IgniteException("RestModule is already started.");
- }
-
- channel = startRestEndpoint(router).channel();
- }
-
- /**
- * Start endpoint.
- *
- * @param router Dispatcher of http requests.
- * @return Future which will be notified when this channel is closed.
- * @throws RuntimeException if this module cannot be bound to a port.
- */
- private ChannelFuture startRestEndpoint(Router router) {
- RestView restConfigurationView = nodeCfgRegistry.getConfiguration(RestConfiguration.KEY).value();
+ RestView restConfigurationView = restConfiguration.value();
int desiredPort = restConfigurationView.port();
int portRange = restConfigurationView.portRange();
- int port = 0;
- Channel ch = null;
-
- ServerBootstrap bootstrap = bootstrapFactory.createServerBootstrap()
- .handler(new LoggingHandler(LogLevel.INFO))
- .childHandler(new RestApiInitializer(router));
-
for (int portCandidate = desiredPort; portCandidate <= desiredPort + portRange; portCandidate++) {
- ChannelFuture bindRes = bootstrap.bind(portCandidate).awaitUninterruptibly();
-
- if (bindRes.isSuccess()) {
- ch = bindRes.channel();
+ try {
+ context = buildMicronautContext(portCandidate).start();
port = portCandidate;
+ log.info("REST protocol started successfully");
+ return;
+ } catch (ApplicationStartupException e) {
+ if (isBindExceptionInside(e)) {
+ log.error("Got exception " + e.getCause().getCause().getCause() + " during node start on port "
+ + portCandidate + " , trying again");
+ continue;
+ }
break;
Review Comment:
Looks like in case of `e` not being a `BindException` you will lose the exception and will not log it. I think it makes sense to simply re-throw it
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r887859742
##########
parent/pom.xml:
##########
@@ -473,6 +475,55 @@
<version>${micronaut.version}</version>
</dependency>
+ <dependency>
+ <groupId>io.micronaut</groupId>
Review Comment:
I don't understand how this can happen. Shouldn't our explicit dependency override what is stated in the bom?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r887328215
##########
modules/cluster-management/src/integrationTest/java/org/apache/ignite/internal/cluster/management/rest/ItClusterManagementControllerTest.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * 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.cluster.management.rest;
+
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import io.micronaut.context.annotation.Bean;
+import io.micronaut.context.annotation.Factory;
+import io.micronaut.context.annotation.Replaces;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpResponse;
+import io.micronaut.http.HttpStatus;
+import io.micronaut.http.client.HttpClient;
+import io.micronaut.http.client.annotation.Client;
+import io.micronaut.http.client.exceptions.HttpClientResponseException;
+import io.micronaut.runtime.server.EmbeddedServer;
+import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
+import jakarta.inject.Inject;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.cluster.management.ClusterInitializer;
+import org.apache.ignite.internal.cluster.management.MockNode;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.apache.ignite.network.ClusterService;
+import org.apache.ignite.network.NetworkAddress;
+import org.apache.ignite.network.StaticNodeFinder;
+import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * Cluster management REST test.
+ */
+@MicronautTest
+@ExtendWith(WorkDirectoryExtension.class)
+public class ItClusterManagementControllerTest {
+
+ private static final int PORT_BASE = 10000;
+
+ private static final List<MockNode> cluster = new ArrayList<>();
+ static ClusterService clusterService;
+ @WorkDirectory
+ private static Path workDir;
+ @Inject
+ EmbeddedServer server;
+ @Inject
+ @Client("/management/v1/cluster/init/")
+ HttpClient client;
+ @Inject
+ ClusterInitializer clusterInitializer;
+
+ @BeforeAll
+ static void setUp(TestInfo testInfo) throws IOException {
+ var addr1 = new NetworkAddress("localhost", PORT_BASE);
+ var addr2 = new NetworkAddress("localhost", PORT_BASE + 1);
+
+ var nodeFinder = new StaticNodeFinder(List.of(addr1, addr2));
+
+ cluster.add(new MockNode(testInfo, addr1, nodeFinder, workDir.resolve("node0")));
+ cluster.add(new MockNode(testInfo, addr2, nodeFinder, workDir.resolve("node1")));
+
+ for (MockNode node : cluster) {
+ node.start();
+ }
+
+ clusterService = cluster.get(0).clusterService();
+ }
+
+ @AfterAll
+ static void tearDown() {
+ for (MockNode node : cluster) {
+ node.beforeNodeStop();
+ }
+
+ for (MockNode node : cluster) {
+ node.stop();
+ }
+ }
+
+ @Test
+ void testControllerLoaded() {
+ assertNotNull(server.getApplicationContext().getBean(ClusterManagementController.class));
+ }
+
+ @Test
+ void testInitNoSuchNode() {
+ // Given body with nodename that does not exist
+ String givenInvalidBody = "{\"metaStorageNodes\": [\"nodename\"], \"cmgNodes\": [], \"clusterName\": \"cluster\"}";
+
+ // When
+ var thrown = assertThrows(
+ HttpClientResponseException.class,
+ () -> client.toBlocking().exchange(HttpRequest.POST("", givenInvalidBody))
+ );
+
+ // Then
+ assertThat(thrown.getResponse().getStatus(), is(equalTo((HttpStatus.BAD_REQUEST))));
+ // And
+ var errorResult = getErrorResult(thrown);
+ assertEquals("INVALID_NODES", errorResult.type());
+ }
+
+ @Test
+ void testInitAlreadyInitializedWithAnotherNodes() {
+ // Given cluster initialized
+ String givenFirstRequestBody =
+ "{\"metaStorageNodes\": [\"" + cluster.get(0).clusterService().localConfiguration().getName() + "\"], \"cmgNodes\": [], "
+ + "\"clusterName\": \"cluster\"}";
+
+ // When
+ HttpResponse<Object> response = client.toBlocking().exchange(HttpRequest.POST("", givenFirstRequestBody));
+
+ // Then
+ assertThat(response.getStatus(), is(equalTo((HttpStatus.OK))));
+ // And
+ assertThat(cluster.get(0).startFuture(), willCompleteSuccessfully());
+
+ // Given second request with different node name
+ String givenSecondRequestBody =
+ "{\"metaStorageNodes\": [\"" + cluster.get(1).clusterService().localConfiguration().getName() + "\"], \"cmgNodes\": [], "
+ + "\"clusterName\": \"cluster\" }";
+
+ // When
+ var thrown = assertThrows(
+ HttpClientResponseException.class,
+ () -> client.toBlocking().exchange(HttpRequest.POST("", givenSecondRequestBody))
+ );
+
+ // Then
+ assertThat(thrown.getResponse().getStatus(), is(equalTo((HttpStatus.CONFLICT))));
+ // And
+ var errorResult = getErrorResult(thrown);
+ assertEquals("ALREADY_INITIALIZED", errorResult.type());
+
+ }
+
+ @Factory
+ @Bean
+ @Replaces(ClusterManagementRestFactory.class)
+ public ClusterManagementRestFactory clusterManagementRestFactory() {
+ return new ClusterManagementRestFactory(clusterService);
+ }
+
+ @NotNull
Review Comment:
> It's not a JSON, but it's not just any plain text. Could we use a specific content type here? For example, here https://www.w3schools.io/file/hocon-introduction/ they suggest application/hocon; it does not seem to be registered with IANA, but it looks logical.
Yes, you are right, that sounds logical. But client generators get confused when you set content type that is not registered with IANA. I tried this.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r884985206
##########
modules/cli/src/main/java/org/apache/ignite/cli/builtins/config/ConfigurationClient.java:
##########
@@ -81,7 +81,7 @@ public String get(
) {
var req = HttpRequest
.newBuilder()
- .header("Content-Type", "application/json");
+ .header("Content-Type", "text/plain");
Review Comment:
Why did you change this?
##########
modules/configuration/src/main/java/org/apache/ignite/internal/rest/configuration/exception/handler/package-info.java:
##########
@@ -0,0 +1,23 @@
+/*
+ * 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.
+ */
+
+/**
+ * This package contains classes that implement the number of ExceptionHandlers. They wrap plain exceptions into human-readable rest
Review Comment:
```suggestion
* This package contains classes that implement a number of ExceptionHandlers. They wrap plain exceptions into human-readable rest
```
##########
modules/rest/pom.xml:
##########
@@ -45,52 +45,41 @@
<dependency>
<groupId>org.apache.ignite</groupId>
- <artifactId>ignite-api</artifactId>
+ <artifactId>ignite-cluster-management</artifactId>
</dependency>
<dependency>
<groupId>org.apache.ignite</groupId>
- <artifactId>ignite-network</artifactId>
+ <artifactId>ignite-api</artifactId>
</dependency>
<!-- 3rd party dependencies -->
+ <!-- micronaut dependencies, use dependencyManagement when ignite-cli is migrated to micronaut 3 -->
Review Comment:
Is there a ticket for this migration?
##########
modules/cluster-management/src/integrationTest/java/org/apache/ignite/internal/cluster/management/ItClusterManagerTest.java:
##########
@@ -107,13 +107,13 @@ void testInit(TestInfo testInfo) throws Exception {
initCluster(metaStorageNodes, cmgNodes);
- assertThat(cluster.get(0).clusterManager().metaStorageNodes(), will(containsInAnyOrder(metaStorageNodes)));
- assertThat(cluster.get(1).clusterManager().metaStorageNodes(), will(containsInAnyOrder(metaStorageNodes)));
+ assertThat(cluster.get(0).getClusterManager().metaStorageNodes(), will(containsInAnyOrder(metaStorageNodes)));
+ assertThat(cluster.get(1).getClusterManager().metaStorageNodes(), will(containsInAnyOrder(metaStorageNodes)));
Review Comment:
why did you change `clusterManager` to `getClusterManager`?
##########
modules/cluster-management/src/integrationTest/java/org/apache/ignite/internal/cluster/management/rest/ClusterManagementControllerTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * 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.cluster.management.rest;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import io.micronaut.context.annotation.Bean;
+import io.micronaut.context.annotation.Factory;
+import io.micronaut.context.annotation.Replaces;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpResponse;
+import io.micronaut.http.HttpStatus;
+import io.micronaut.http.client.HttpClient;
+import io.micronaut.http.client.annotation.Client;
+import io.micronaut.http.client.exceptions.HttpClientResponseException;
+import io.micronaut.runtime.server.EmbeddedServer;
+import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
+import jakarta.inject.Inject;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.cluster.management.ClusterInitializer;
+import org.apache.ignite.internal.cluster.management.MockNode;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.apache.ignite.network.ClusterService;
+import org.apache.ignite.network.NetworkAddress;
+import org.apache.ignite.network.StaticNodeFinder;
+import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * Cluster management REST test.
+ */
+@MicronautTest
+@ExtendWith(WorkDirectoryExtension.class)
+public class ClusterManagementControllerTest {
+
+ private static final int PORT_BASE = 10000;
+
+ private static final List<MockNode> cluster = new ArrayList<>();
+ static ClusterService clusterService;
+ @WorkDirectory
+ private static Path workDir;
+ @Inject
+ EmbeddedServer server;
+ @Inject
+ @Client("/management/v1/cluster/init/")
+ HttpClient client;
+ @Inject
+ ClusterInitializer clusterInitializer;
+
+ @BeforeAll
+ static void setUp(TestInfo testInfo) throws IOException {
+ var addr1 = new NetworkAddress("localhost", PORT_BASE);
+ var addr2 = new NetworkAddress("localhost", PORT_BASE + 1);
+
+ var nodeFinder = new StaticNodeFinder(List.of(addr1, addr2));
+
+ cluster.add(new MockNode(testInfo, addr1, nodeFinder, workDir.resolve("node0")));
+ cluster.add(new MockNode(testInfo, addr2, nodeFinder, workDir.resolve("node1")));
+
+ for (MockNode node : cluster) {
+ node.start();
+ }
+
+ clusterService = cluster.get(0).getClusterService();
+
+ }
+
+ @AfterAll
+ static void tearDown() {
+ for (MockNode node : cluster) {
+ node.beforeNodeStop();
+ }
+
+ for (MockNode node : cluster) {
+ node.stop();
+ }
+ }
+
+ @Test
+ void testControllerLoaded() {
+ assertNotNull(server.getApplicationContext().getBean(ClusterManagementController.class));
+ }
+
+ @Test
+ void testInitNoSuchNode() {
+ // Given body with nodename that does not exist
+ String givenInvalidBody = "{\"metaStorageNodes\": [\"nodename\"], \"cmgNodes\": [], \"clusterName\": \"cluster\"}";
+
+ try {
+ // When
+ client.toBlocking().exchange(HttpRequest.POST("", givenInvalidBody));
+ } catch (HttpClientResponseException e) {
+ // Then
+ assertThat(e.getResponse().getStatus(), is(equalTo((HttpStatus.BAD_REQUEST))));
+ // And
+ var errorResult = getErrorResult(e);
+ assertEquals("INVALID_NODES", errorResult.type());
+ }
+ }
+
+ @Test
+ void testInitAlreadyInitializedWithAnotherNodes() {
+ // Given cluster initialized
+ String givenFirstRequestBody =
+ "{\"metaStorageNodes\": [\"" + cluster.get(0).getClusterService().localConfiguration().getName() + "\"], \"cmgNodes\": [], "
+ + "\"clusterName\": \"cluster\"}";
+ // When
+ HttpResponse<Object> response = client.toBlocking().exchange(HttpRequest.POST("", givenFirstRequestBody));
+ // Then
+ assertThat(response.getStatus(), is(equalTo((HttpStatus.OK))));
+
+ // And second request with different node name
+ String givenSecondRequestBody =
+ "{\"metaStorageNodes\": [\"" + cluster.get(1).getClusterService().localConfiguration().getName() + "\"], \"cmgNodes\": [], "
+ + "\"clusterName\": \"cluster\" }";
+
+ try {
+ // When
+ client.toBlocking().exchange(HttpRequest.POST("", givenSecondRequestBody));
+ } catch (HttpClientResponseException e) {
+ // Then
+ assertThat(e.getResponse().getStatus(), is(equalTo((HttpStatus.CONFLICT))));
+ // And
+ var errorResult = getErrorResult(e);
+ assertEquals("ALREADY_INITIALIZED", errorResult.type());
+ }
+ }
+
+ @Factory
+ @Bean
+ @Replaces(ClusterManagementRestFactory.class)
+ public ClusterManagementRestFactory clusterManagementRestFactory() {
+ return new ClusterManagementRestFactory(clusterService);
+ }
+
+ @NotNull
+ private ErrorResult getErrorResult(HttpClientResponseException exception) {
+ return exception.getResponse().getBody(ErrorResult.class).orElseThrow();
+ }
+}
Review Comment:
Line break is missing, please check your IDE settings
##########
modules/rest-api/pom.xml:
##########
@@ -18,70 +18,57 @@
-->
<project xmlns="http://maven.apache.org/POM/4.0.0"
- xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
Review Comment:
Please check your formatting settings, this file is all messed up
##########
modules/configuration/src/main/java/org/apache/ignite/internal/rest/configuration/NodeConfigurationController.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.rest.configuration;
+
+import io.micronaut.http.MediaType;
+import io.micronaut.http.annotation.Body;
+import io.micronaut.http.annotation.Consumes;
+import io.micronaut.http.annotation.Controller;
+import io.micronaut.http.annotation.Get;
+import io.micronaut.http.annotation.Patch;
+import io.micronaut.http.annotation.PathVariable;
+import io.micronaut.http.annotation.Produces;
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.media.Content;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import jakarta.inject.Named;
+import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation;
+
+/**
+ * Node configuration controller.
+ */
+@Controller("/management/v1/configuration/node")
+@ApiResponse(responseCode = "400", description = "Incorrect configuration")
+@ApiResponse(responseCode = "500", description = "Internal error")
+@Tag(name = "nodeConfiguration")
+public class NodeConfigurationController extends AbstractConfigurationController {
+
+ public NodeConfigurationController(@Named("nodeCfgPresentation") ConfigurationPresentation<String> nodeCfgPresentation) {
+ super(nodeCfgPresentation);
+ }
+
+ /**
+ * Returns node configuration in HOCON format.
+ *
+ * @return the whole node configuration in HOCON format.
+ */
+ @Operation(operationId = "getNodeConfiguration")
+ @ApiResponse(responseCode = "200",
+ content = @Content(mediaType = MediaType.TEXT_PLAIN,
+ schema = @Schema(type = "string")),
+ description = "Whole node configuration")
+ @Produces(MediaType.TEXT_PLAIN)
+ @Get
+ public String getConfiguration() {
+ return super.getConfiguration();
+ }
+
+ /**
+ * Returns configuration in HOCON format represented by path.
+ *
+ * @param path to represent a node configuration.
+ * @return system configuration in HOCON format represented by given path.
+ */
+ @Operation(operationId = "getNodeConfigurationByPath")
+ @ApiResponse(responseCode = "200",
+ content = @Content(mediaType = MediaType.TEXT_PLAIN,
+ schema = @Schema(type = "string")),
+ description = "Configuration represented by path")
+ @Produces(MediaType.TEXT_PLAIN)
+ @Get("/{path}")
+ public String getConfigurationByPath(@PathVariable String path) {
+ return super.getConfigurationByPath(path);
+ }
+
+ /**
+ * Updates node configuration in HOCON format.
+ *
+ * @param updatedConfiguration the node configuration to update.
+ */
+ @Operation(operationId = "updateNodeConfiguration")
+ @ApiResponse(responseCode = "200", description = "Configuration updated")
+ @Consumes(MediaType.TEXT_PLAIN)
+ @Patch
+ public void updateConfiguration(@Body String updatedConfiguration) throws Throwable {
Review Comment:
Same comment as in `ClusterConfigurationController`
##########
modules/rest-api/src/main/java/org/apache/ignite/internal/rest/api/RestFactory.java:
##########
@@ -15,8 +15,10 @@
* limitations under the License.
*/
+package org.apache.ignite.internal.rest.api;
+
/**
- * This package contains classes that represent inbound requests, outbound responses and main REST handler.
+ * Factory that produces all beans that is necessary for the controller class.
*/
-
-package org.apache.ignite.internal.rest.netty;
+public interface RestFactory {
Review Comment:
Why do you need this interface?
##########
modules/configuration/src/main/java/org/apache/ignite/internal/rest/configuration/PresentationsFactory.java:
##########
@@ -0,0 +1,55 @@
+/*
+ * 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.rest.configuration;
+
+import io.micronaut.context.annotation.Bean;
+import io.micronaut.context.annotation.Factory;
+import jakarta.inject.Named;
+import jakarta.inject.Singleton;
+import org.apache.ignite.internal.configuration.ConfigurationManager;
+import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation;
+import org.apache.ignite.internal.configuration.rest.presentation.hocon.HoconPresentation;
+import org.apache.ignite.internal.rest.api.RestFactory;
+
+/**
+ * Factory that defines beans that needed for rest module.
Review Comment:
```suggestion
* Factory that defines beans required for the rest module.
```
##########
modules/configuration/src/main/java/org/apache/ignite/internal/rest/configuration/ClusterConfigurationController.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.rest.configuration;
+
+import io.micronaut.http.MediaType;
+import io.micronaut.http.annotation.Body;
+import io.micronaut.http.annotation.Consumes;
+import io.micronaut.http.annotation.Controller;
+import io.micronaut.http.annotation.Get;
+import io.micronaut.http.annotation.Patch;
+import io.micronaut.http.annotation.PathVariable;
+import io.micronaut.http.annotation.Produces;
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.media.Content;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import jakarta.inject.Named;
+import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation;
+
+/**
+ * Cluster configuration controller.
+ */
+@Controller("/management/v1/configuration/cluster/")
+@ApiResponse(responseCode = "400", description = "Incorrect configuration")
+@ApiResponse(responseCode = "500", description = "Internal error")
+@Tag(name = "clusterConfiguration")
+public class ClusterConfigurationController extends AbstractConfigurationController {
+
+ public ClusterConfigurationController(@Named("clusterCfgPresentation") ConfigurationPresentation<String> clusterCfgPresentation) {
+ super(clusterCfgPresentation);
+ }
+
+ /**
+ * Returns cluster configuration in HOCON format.
+ *
+ * @return the whole cluster configuration in HOCON format.
+ */
+ @Operation(operationId = "getClusterConfiguration")
+ @ApiResponse(
+ responseCode = "200",
+ content = @Content(mediaType = MediaType.TEXT_PLAIN, schema = @Schema(type = "string")),
+ description = "Get cluster configuration")
+ @Produces(MediaType.TEXT_PLAIN)
+ @Get
+ public String getConfiguration() {
+ return super.getConfiguration();
+ }
+
+ /**
+ * Returns configuration in HOCON format represented by path.
+ *
+ * @param path to represent a cluster configuration.
+ * @return system configuration in HOCON format represented by given path.
+ */
+ @Operation(operationId = "getClusterConfigurationByPath")
+ @ApiResponse(responseCode = "200",
+ content = @Content(mediaType = MediaType.TEXT_PLAIN,
+ schema = @Schema(type = "string")),
+ description = "Configuration represented by path")
+ @Produces(MediaType.TEXT_PLAIN)
+ @Get("/{path}")
+ public String getConfigurationByPath(@PathVariable String path) {
+ return super.getConfigurationByPath(path);
+ }
+
+ /**
+ * Updates cluster configuration in HOCON format.
+ *
+ * @param updatedConfiguration the cluster configuration to update.
+ */
+ @Operation(operationId = "updateClusterConfiguration")
+ @ApiResponse(responseCode = "200", description = "Configuration updated")
+ @Consumes(MediaType.TEXT_PLAIN)
+ @Patch
+ public void updateConfiguration(@Body String updatedConfiguration) throws Throwable {
Review Comment:
I believe it's possible to return `CompletableFuture` as a response in Micronaut controller. Can we do that? If yes. I would also make `updateConfiguration` non-blockable
##########
parent/pom.xml:
##########
@@ -954,6 +963,7 @@
<ignorePackages>
<package>org.apache.ignite.internal.sql.engine</package>
<package>org.apache.ignite.internal.generated</package>
+ <package>org.apache.ignite.rest.client</package>
Review Comment:
Do I understand correctly that this package contains clients generated by Swagger? If yes, please add a comment, if no - why did you add it here?
##########
parent/pom.xml:
##########
@@ -53,19 +53,21 @@
<asm.framework.version>9.0</asm.framework.version>
<compile.testing.library.version>0.19</compile.testing.library.version>
<jackson.version>2.13.1</jackson.version>
+ <jakarta.annotations.version>2.0.0</jakarta.annotations.version>
<jansi.version>1.18</jansi.version>
<netty.version>4.1.70.Final</netty.version>
<javapoet.version>1.13.0</javapoet.version>
<javax.annotation.api.version>1.3.2</javax.annotation.api.version>
<jetbrains.annotations.version>20.1.0</jetbrains.annotations.version>
<jmh.framework.version>1.35</jmh.framework.version>
<junit.version>5.8.1</junit.version>
- <micronaut.version>2.1.2</micronaut.version>
- <micronaut.test.junit5.version>2.3.1</micronaut.test.junit5.version>
+ <micronaut.version>3.4.1</micronaut.version>
+ <micronaut.test.junit5.version>3.0.5</micronaut.test.junit5.version>
Review Comment:
I think we can get rid of this property if we use a bom
##########
modules/configuration/src/main/java/org/apache/ignite/internal/rest/configuration/ClusterConfigurationController.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.rest.configuration;
+
+import io.micronaut.http.MediaType;
+import io.micronaut.http.annotation.Body;
+import io.micronaut.http.annotation.Consumes;
+import io.micronaut.http.annotation.Controller;
+import io.micronaut.http.annotation.Get;
+import io.micronaut.http.annotation.Patch;
+import io.micronaut.http.annotation.PathVariable;
+import io.micronaut.http.annotation.Produces;
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.media.Content;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import jakarta.inject.Named;
+import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation;
+
+/**
+ * Cluster configuration controller.
+ */
+@Controller("/management/v1/configuration/cluster/")
+@ApiResponse(responseCode = "400", description = "Incorrect configuration")
+@ApiResponse(responseCode = "500", description = "Internal error")
+@Tag(name = "clusterConfiguration")
+public class ClusterConfigurationController extends AbstractConfigurationController {
+
+ public ClusterConfigurationController(@Named("clusterCfgPresentation") ConfigurationPresentation<String> clusterCfgPresentation) {
+ super(clusterCfgPresentation);
+ }
+
+ /**
+ * Returns cluster configuration in HOCON format.
+ *
+ * @return the whole cluster configuration in HOCON format.
+ */
+ @Operation(operationId = "getClusterConfiguration")
+ @ApiResponse(
+ responseCode = "200",
+ content = @Content(mediaType = MediaType.TEXT_PLAIN, schema = @Schema(type = "string")),
+ description = "Get cluster configuration")
+ @Produces(MediaType.TEXT_PLAIN)
Review Comment:
Why not `application/json`?
##########
modules/rest-client/pom.xml:
##########
@@ -0,0 +1,124 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ 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.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <parent>
+ <groupId>org.apache.ignite</groupId>
+ <artifactId>ignite-parent</artifactId>
+ <version>1</version>
+ <relativePath>../../parent/pom.xml</relativePath>
+ </parent>
+
+ <artifactId>ignite-rest-client</artifactId>
+ <version>3.0.0-SNAPSHOT</version>
+
+ <dependencies>
+
+ <!--client specific dependencies, versions cannot be moved to the parent pom-->
Review Comment:
Is this module (and this file) auto-generated?
##########
modules/cluster-management/pom.xml:
##########
@@ -49,17 +49,26 @@
<dependency>
<groupId>org.apache.ignite</groupId>
- <artifactId>ignite-rest</artifactId>
+ <artifactId>ignite-vault</artifactId>
</dependency>
<dependency>
<groupId>org.apache.ignite</groupId>
- <artifactId>ignite-vault</artifactId>
+ <artifactId>ignite-rest-api</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>io.micronaut</groupId>
+ <artifactId>micronaut-http</artifactId>
+ <version>${micronaut.version}</version>
Review Comment:
Shall we use [micronaut-bom](https://mvnrepository.com/artifact/io.micronaut/micronaut-bom) instead of specifying `micronaut.version` explicitly?
##########
modules/cluster-management/pom.xml:
##########
@@ -49,17 +49,26 @@
<dependency>
<groupId>org.apache.ignite</groupId>
- <artifactId>ignite-rest</artifactId>
+ <artifactId>ignite-vault</artifactId>
</dependency>
<dependency>
<groupId>org.apache.ignite</groupId>
- <artifactId>ignite-vault</artifactId>
+ <artifactId>ignite-rest-api</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>io.micronaut</groupId>
+ <artifactId>micronaut-http</artifactId>
+ <version>${micronaut.version}</version>
Review Comment:
Also, I believe that `compile` scope is the default, so no need to specify it
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r885504526
##########
modules/cli/src/main/java/org/apache/ignite/cli/builtins/config/ConfigurationClient.java:
##########
@@ -81,7 +81,7 @@ public String get(
) {
var req = HttpRequest
.newBuilder()
- .header("Content-Type", "application/json");
+ .header("Content-Type", "text/plain");
Review Comment:
Sure.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r887879767
##########
parent/pom.xml:
##########
@@ -473,6 +475,55 @@
<version>${micronaut.version}</version>
</dependency>
+ <dependency>
+ <groupId>io.micronaut</groupId>
Review Comment:
junit-bom does not override this
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r886902594
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java:
##########
@@ -17,151 +17,147 @@
package org.apache.ignite.internal.rest;
-import io.netty.bootstrap.ServerBootstrap;
-import io.netty.channel.Channel;
-import io.netty.channel.ChannelFuture;
-import io.netty.handler.logging.LogLevel;
-import io.netty.handler.logging.LoggingHandler;
+import io.micronaut.context.ApplicationContext;
+import io.micronaut.http.server.exceptions.ServerStartupException;
+import io.micronaut.openapi.annotation.OpenAPIInclude;
+import io.micronaut.runtime.Micronaut;
+import io.micronaut.runtime.exceptions.ApplicationStartupException;
+import io.swagger.v3.oas.annotations.OpenAPIDefinition;
+import io.swagger.v3.oas.annotations.info.Contact;
+import io.swagger.v3.oas.annotations.info.Info;
+import io.swagger.v3.oas.annotations.info.License;
import java.net.BindException;
-import java.net.InetSocketAddress;
-import java.util.function.Consumer;
+import java.util.List;
+import java.util.Map;
import org.apache.ignite.configuration.schemas.rest.RestConfiguration;
import org.apache.ignite.configuration.schemas.rest.RestView;
-import org.apache.ignite.internal.configuration.ConfigurationManager;
-import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController;
import org.apache.ignite.internal.manager.IgniteComponent;
-import org.apache.ignite.internal.rest.api.RestHandlersRegister;
-import org.apache.ignite.internal.rest.api.Routes;
-import org.apache.ignite.internal.rest.netty.RestApiInitializer;
-import org.apache.ignite.internal.rest.routes.Router;
-import org.apache.ignite.internal.rest.routes.SimpleRouter;
-import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.internal.rest.api.RestFactory;
+import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController;
+import org.apache.ignite.internal.rest.configuration.NodeConfigurationController;
import org.apache.ignite.lang.IgniteInternalException;
import org.apache.ignite.lang.IgniteLogger;
-import org.apache.ignite.network.NettyBootstrapFactory;
/**
- * Rest component is responsible for starting REST endpoints.
+ * Rest module is responsible for starting a REST endpoints for accessing and managing configuration.
*
- * <p>It is started on port 10300 by default, but it is possible to change this in configuration itself. Refer to default config file in
+ * <p>It is started on port 10300 by default but it is possible to change this in configuration itself. Refer to default config file in
* resources for the example.
*/
-public class RestComponent implements RestHandlersRegister, IgniteComponent {
+@OpenAPIDefinition(info = @Info(title = "Ignite REST module", version = "3.0.0-alpha", license = @License(name = "Apache 2.0", url = "https://ignite.apache.org"), contact = @Contact(email = "user@ignite.apache.org")))
+@OpenAPIInclude(classes = {ClusterConfigurationController.class, NodeConfigurationController.class, ClusterManagementController.class})
+public class RestComponent implements IgniteComponent {
+ /** Default port. */
+ public static final int DFLT_PORT = 10300;
+
/** Ignite logger. */
private final IgniteLogger log = IgniteLogger.forClass(RestComponent.class);
- /** Node configuration register. */
- private final ConfigurationRegistry nodeCfgRegistry;
-
- /** Netty bootstrap factory. */
- private final NettyBootstrapFactory bootstrapFactory;
-
- private final SimpleRouter router = new SimpleRouter();
-
- /** Netty channel. */
- private volatile Channel channel;
+ /** Factories that produce beans needed for REST controllers. */
+ private final List<RestFactory> restFactories;
+ private final RestConfiguration restConfiguration;
+ /** Server host. */
+ private final String host = "localhost";
+ /** Micronaut application context. */
+ private ApplicationContext context;
Review Comment:
Good point.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r885531194
##########
modules/configuration/src/main/java/org/apache/ignite/internal/rest/configuration/ClusterConfigurationController.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.rest.configuration;
+
+import io.micronaut.http.MediaType;
+import io.micronaut.http.annotation.Body;
+import io.micronaut.http.annotation.Consumes;
+import io.micronaut.http.annotation.Controller;
+import io.micronaut.http.annotation.Get;
+import io.micronaut.http.annotation.Patch;
+import io.micronaut.http.annotation.PathVariable;
+import io.micronaut.http.annotation.Produces;
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.media.Content;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import jakarta.inject.Named;
+import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation;
+
+/**
+ * Cluster configuration controller.
+ */
+@Controller("/management/v1/configuration/cluster/")
+@ApiResponse(responseCode = "400", description = "Incorrect configuration")
+@ApiResponse(responseCode = "500", description = "Internal error")
+@Tag(name = "clusterConfiguration")
+public class ClusterConfigurationController extends AbstractConfigurationController {
+
+ public ClusterConfigurationController(@Named("clusterCfgPresentation") ConfigurationPresentation<String> clusterCfgPresentation) {
+ super(clusterCfgPresentation);
+ }
+
+ /**
+ * Returns cluster configuration in HOCON format.
+ *
+ * @return the whole cluster configuration in HOCON format.
+ */
+ @Operation(operationId = "getClusterConfiguration")
+ @ApiResponse(
+ responseCode = "200",
+ content = @Content(mediaType = MediaType.TEXT_PLAIN, schema = @Schema(type = "string")),
+ description = "Get cluster configuration")
+ @Produces(MediaType.TEXT_PLAIN)
Review Comment:
I'll add a comment.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ptupitsyn commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
ptupitsyn commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r884931683
##########
modules/configuration/src/test/java/org/apache/ignite/internal/rest/configuration/ConfigurationControllerBaseTest.java:
##########
@@ -0,0 +1,149 @@
+/*
+ * 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.rest.configuration;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import io.micronaut.context.ApplicationContext;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpStatus;
+import io.micronaut.http.MediaType;
+import io.micronaut.http.client.HttpClient;
+import io.micronaut.http.client.exceptions.HttpClientResponseException;
+import io.micronaut.runtime.server.EmbeddedServer;
+import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
+import jakarta.inject.Inject;
+import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation;
+import org.apache.ignite.internal.configuration.rest.presentation.TestRootConfiguration;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * The base test for configuration controllers.
+ */
+@MicronautTest
+public abstract class ConfigurationControllerBaseTest {
+ @Inject
+ EmbeddedServer server;
+
+ @Inject
+ ConfigurationPresentation<String> cfgPresentation;
+
+ @Inject
+ ConfigurationRegistry configurationRegistry;
+
+ @Inject
+ ApplicationContext context;
+
+ abstract HttpClient client();
+
+ @BeforeEach
+ void beforeEach() throws Exception {
+ var cfg = configurationRegistry.getConfiguration(TestRootConfiguration.KEY);
+ cfg.change(c -> c.changeFoo("foo").changeSubCfg(subCfg -> subCfg.changeBar("bar"))).get(1, SECONDS);
+ }
+
+ @Test
+ void testGetConfig() {
+ var response = client().toBlocking().exchange("", String.class);
+
+ assertEquals(HttpStatus.OK, response.status());
+ assertEquals(cfgPresentation.represent(), response.body());
+ }
+
+ @Test
+ void testGetConfigByPath() {
+ var response = client().toBlocking().exchange("/root.subCfg", String.class);
+
+ assertEquals(HttpStatus.OK, response.status());
+ assertEquals(cfgPresentation.representByPath("root.subCfg"), response.body());
+ }
+
+ @Test
+ void testUpdateConfig() {
+ String givenChangedConfig = "{root:{foo:foo,subCfg:{bar:changed}}}";
+
+ var response = client().toBlocking().exchange(
+ HttpRequest.PATCH("", givenChangedConfig).contentType(MediaType.TEXT_PLAIN)
+ );
+ assertEquals(response.status(), HttpStatus.OK);
+
+ String changedConfigValue = client().toBlocking().exchange("/root.subCfg.bar", String.class).body();
+ assertEquals("\"changed\"", changedConfigValue);
+ }
+
+ @Test
+ void testUnrecognizedConfigPath() {
+ try {
+ client().toBlocking().exchange("/no-such-root.some-value");
+ fail("Expected exception to be thrown");
+ } catch (HttpClientResponseException exception) {
+ exception.printStackTrace();
+ assertEquals(HttpStatus.BAD_REQUEST, exception.getResponse().status());
+
+ var errorResult = getErrorResult(exception);
+ assertEquals("CONFIG_PATH_UNRECOGNIZED", errorResult.type());
+ assertTrue(errorResult.message().contains("no-such-root"));
+ }
+ }
+
+ @Test
+ void testUnrecognizedConfigPathForUpdate() {
+ String givenBrokenConfig = "{root:{foo:foo,subCfg:{no-such-bar:bar}}}";
+ try {
+ client().toBlocking().exchange(
+ HttpRequest.PATCH("", givenBrokenConfig).contentType(MediaType.TEXT_PLAIN)
+ );
+ fail("Expected exception to be thrown");
+ } catch (HttpClientResponseException exception) {
+ assertEquals(HttpStatus.BAD_REQUEST, exception.getResponse().status());
+
+ var errorResult = getErrorResult(exception);
+ assertEquals("INVALID_CONFIG_FORMAT", errorResult.type());
+ assertTrue(errorResult.message().contains("no-such-bar"));
+ }
+ }
+
+ @Test
+ void testValidationForUpdate() {
+ String givenConfigWithError = "{root:{foo:error,subCfg:{bar:bar}}}";
+ try {
+ client().toBlocking().exchange(
+ HttpRequest.PATCH("", givenConfigWithError).contentType(MediaType.TEXT_PLAIN)
+ );
+ fail("Expected exception to be thrown");
Review Comment:
Here and other tests: prefer `assertThrows` instead of catching exceptions.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r887845060
##########
parent/pom.xml:
##########
@@ -473,6 +475,55 @@
<version>${micronaut.version}</version>
</dependency>
+ <dependency>
+ <groupId>io.micronaut</groupId>
Review Comment:
micronaut-bom downgrades <junit5.version>5.7.2</junit5.version> , but ignite 3 uses 5.8.1 version. It actually breaks the compilation. Are we ok to downgrade the junit version?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r887825449
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java:
##########
@@ -17,151 +17,161 @@
package org.apache.ignite.internal.rest;
-import io.netty.bootstrap.ServerBootstrap;
-import io.netty.channel.Channel;
-import io.netty.channel.ChannelFuture;
-import io.netty.handler.logging.LogLevel;
-import io.netty.handler.logging.LoggingHandler;
+import io.micronaut.context.ApplicationContext;
+import io.micronaut.http.server.exceptions.ServerStartupException;
+import io.micronaut.openapi.annotation.OpenAPIInclude;
+import io.micronaut.runtime.Micronaut;
+import io.micronaut.runtime.exceptions.ApplicationStartupException;
+import io.swagger.v3.oas.annotations.OpenAPIDefinition;
+import io.swagger.v3.oas.annotations.info.Contact;
+import io.swagger.v3.oas.annotations.info.Info;
+import io.swagger.v3.oas.annotations.info.License;
import java.net.BindException;
-import java.net.InetSocketAddress;
-import java.util.function.Consumer;
+import java.util.List;
+import java.util.Map;
import org.apache.ignite.configuration.schemas.rest.RestConfiguration;
import org.apache.ignite.configuration.schemas.rest.RestView;
-import org.apache.ignite.internal.configuration.ConfigurationManager;
-import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController;
import org.apache.ignite.internal.manager.IgniteComponent;
-import org.apache.ignite.internal.rest.api.RestHandlersRegister;
-import org.apache.ignite.internal.rest.api.Routes;
-import org.apache.ignite.internal.rest.netty.RestApiInitializer;
-import org.apache.ignite.internal.rest.routes.Router;
-import org.apache.ignite.internal.rest.routes.SimpleRouter;
-import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.internal.rest.api.RestFactory;
+import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController;
+import org.apache.ignite.internal.rest.configuration.NodeConfigurationController;
import org.apache.ignite.lang.IgniteInternalException;
import org.apache.ignite.lang.IgniteLogger;
-import org.apache.ignite.network.NettyBootstrapFactory;
/**
- * Rest component is responsible for starting REST endpoints.
+ * Rest module is responsible for starting REST endpoints for accessing and managing configuration.
*
- * <p>It is started on port 10300 by default, but it is possible to change this in configuration itself. Refer to default config file in
+ * <p>It is started on port 10300 by default but it is possible to change this in configuration itself. Refer to default config file in
* resources for the example.
*/
-public class RestComponent implements RestHandlersRegister, IgniteComponent {
+@OpenAPIDefinition(info = @Info(title = "Ignite REST module", version = "3.0.0-alpha", license = @License(name = "Apache 2.0", url = "https://ignite.apache.org"), contact = @Contact(email = "user@ignite.apache.org")))
+@OpenAPIInclude(classes = {ClusterConfigurationController.class, NodeConfigurationController.class, ClusterManagementController.class})
+public class RestComponent implements IgniteComponent {
+ /** Default port. */
+ public static final int DFLT_PORT = 10300;
+
+ /** Server host. */
+ private static final String LOCALHOST = "localhost";
+
/** Ignite logger. */
private final IgniteLogger log = IgniteLogger.forClass(RestComponent.class);
- /** Node configuration register. */
- private final ConfigurationRegistry nodeCfgRegistry;
+ /** Factories that produce beans needed for REST controllers. */
+ private final List<RestFactory> restFactories;
- /** Netty bootstrap factory. */
- private final NettyBootstrapFactory bootstrapFactory;
+ private final RestConfiguration restConfiguration;
- private final SimpleRouter router = new SimpleRouter();
+ /** Micronaut application context. */
+ private volatile ApplicationContext context;
- /** Netty channel. */
- private volatile Channel channel;
+ /** Server port. */
+ private int port;
/**
* Creates a new instance of REST module.
- *
- * @param nodeCfgMgr Node configuration manager.
- * @param bootstrapFactory Netty bootstrap factory.
*/
- public RestComponent(ConfigurationManager nodeCfgMgr, NettyBootstrapFactory bootstrapFactory) {
- nodeCfgRegistry = nodeCfgMgr.configurationRegistry();
-
- this.bootstrapFactory = bootstrapFactory;
- }
-
- /** {@inheritDoc} */
- @Override
- public void registerHandlers(Consumer<Routes> registerAction) {
- registerAction.accept(router);
+ public RestComponent(List<RestFactory> restFactories, RestConfiguration restConfiguration) {
+ this.restFactories = restFactories;
+ this.restConfiguration = restConfiguration;
}
/** {@inheritDoc} */
@Override
public void start() {
- if (channel != null) {
- throw new IgniteException("RestModule is already started.");
- }
-
- channel = startRestEndpoint(router).channel();
- }
-
- /**
- * Start endpoint.
- *
- * @param router Dispatcher of http requests.
- * @return Future which will be notified when this channel is closed.
- * @throws RuntimeException if this module cannot be bound to a port.
- */
- private ChannelFuture startRestEndpoint(Router router) {
- RestView restConfigurationView = nodeCfgRegistry.getConfiguration(RestConfiguration.KEY).value();
+ RestView restConfigurationView = restConfiguration.value();
int desiredPort = restConfigurationView.port();
int portRange = restConfigurationView.portRange();
- int port = 0;
- Channel ch = null;
-
- ServerBootstrap bootstrap = bootstrapFactory.createServerBootstrap()
- .handler(new LoggingHandler(LogLevel.INFO))
- .childHandler(new RestApiInitializer(router));
-
for (int portCandidate = desiredPort; portCandidate <= desiredPort + portRange; portCandidate++) {
- ChannelFuture bindRes = bootstrap.bind(portCandidate).awaitUninterruptibly();
-
- if (bindRes.isSuccess()) {
- ch = bindRes.channel();
+ try {
+ context = buildMicronautContext(portCandidate).start();
port = portCandidate;
Review Comment:
Thanks, you are absolutelly right.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r886843611
##########
modules/cli/src/integrationTest/resources/hardcoded-ports-config.json:
##########
@@ -8,6 +8,6 @@
},
"rest": {
"port": <REST_PORT>,
- "portRange": 0
+ "portRange": 5
Review Comment:
Reverted.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r887821372
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalExceptionHandler.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * 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.cluster.management.rest.exception.handler;
+
+import io.micronaut.context.annotation.Requires;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpResponse;
+import io.micronaut.http.annotation.Produces;
+import io.micronaut.http.server.exceptions.ExceptionHandler;
+import jakarta.inject.Singleton;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.apache.ignite.lang.IgniteInternalException;
+
+/**
+ * Handles {@link IgniteInternalException} and represents it as a rest response.
+ */
+@Produces
+@Singleton
+@Requires(classes = {IgniteInternalException.class, ExceptionHandler.class})
+public class IgniteInternalExceptionHandler implements ExceptionHandler<IgniteInternalException, HttpResponse<ErrorResult>> {
+
+ @Override
+ public HttpResponse<ErrorResult> handle(HttpRequest request, IgniteInternalException exception) {
+ ErrorResult errorResult = new ErrorResult("INITIALIZATION_EXCEPTION", exception.getMessage());
+ return HttpResponse.badRequest().body(errorResult);
Review Comment:
Not it is not obvious what exactly happend on IgniteInternalException. `IgniteInternalException` looks like 500 and I think it is better to mark it as an internal server error.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r886835634
##########
modules/rest-api/src/main/java/org/apache/ignite/internal/rest/api/RestFactory.java:
##########
@@ -15,8 +15,10 @@
* limitations under the License.
*/
+package org.apache.ignite.internal.rest.api;
+
/**
- * This package contains classes that represent inbound requests, outbound responses and main REST handler.
+ * Factory that produces all beans that is necessary for the controller class.
*/
-
-package org.apache.ignite.internal.rest.netty;
+public interface RestFactory {
Review Comment:
Got it, thank you
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r885084534
##########
modules/rest/pom.xml:
##########
@@ -45,52 +45,41 @@
<dependency>
<groupId>org.apache.ignite</groupId>
- <artifactId>ignite-api</artifactId>
+ <artifactId>ignite-cluster-management</artifactId>
</dependency>
<dependency>
<groupId>org.apache.ignite</groupId>
- <artifactId>ignite-network</artifactId>
+ <artifactId>ignite-api</artifactId>
</dependency>
<!-- 3rd party dependencies -->
+ <!-- micronaut dependencies, use dependencyManagement when ignite-cli is migrated to micronaut 3 -->
Review Comment:
I am going to apply this change in the current pull request. In case I face some problem I'll create one.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r885084701
##########
parent/pom.xml:
##########
@@ -954,6 +963,7 @@
<ignorePackages>
<package>org.apache.ignite.internal.sql.engine</package>
<package>org.apache.ignite.internal.generated</package>
+ <package>org.apache.ignite.rest.client</package>
Review Comment:
Yes, you are right. I'll add a comment.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r887887346
##########
parent/pom.xml:
##########
@@ -473,6 +475,55 @@
<version>${micronaut.version}</version>
</dependency>
+ <dependency>
+ <groupId>io.micronaut</groupId>
Review Comment:
indeed, I forgot that we are using junit as a bom too. Ok, let's dump this idea then
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r887852595
##########
modules/cluster-management/pom.xml:
##########
@@ -49,17 +49,26 @@
<dependency>
<groupId>org.apache.ignite</groupId>
- <artifactId>ignite-rest</artifactId>
+ <artifactId>ignite-vault</artifactId>
</dependency>
<dependency>
<groupId>org.apache.ignite</groupId>
- <artifactId>ignite-vault</artifactId>
+ <artifactId>ignite-rest-api</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>io.micronaut</groupId>
+ <artifactId>micronaut-http</artifactId>
+ <version>${micronaut.version}</version>
Review Comment:
What about two boms have different versions?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
rpuch commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r886896182
##########
modules/cli/src/integrationTest/java/org/apache/ignite/cli/ItConfigCommandTest.java:
##########
@@ -113,9 +109,7 @@ public void setAndGetWithManualHost() {
assertEquals(0, exitCode);
- DocumentContext document = JsonPath.parse(removeTrailingQuotes(unescapeQuotes(out.toString(UTF_8))));
-
- assertEquals(1, document.read("$.network.shutdownQuietPeriod", Integer.class));
+ assertTrue(out.toString(UTF_8).contains("\"shutdownQuietPeriod\" : 1"));
Review Comment:
I suggest using Hamcrest assertions, like this:
```
assertThat(out.toString(UTF_8), containsString(...));
```
for such assertions. They produce more useful error messages on failures that make it easier to understand what was expected and what actually happened.
##########
modules/cluster-management/src/integrationTest/java/org/apache/ignite/internal/cluster/management/rest/ItClusterManagementControllerTest.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * 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.cluster.management.rest;
+
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import io.micronaut.context.annotation.Bean;
+import io.micronaut.context.annotation.Factory;
+import io.micronaut.context.annotation.Replaces;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpResponse;
+import io.micronaut.http.HttpStatus;
+import io.micronaut.http.client.HttpClient;
+import io.micronaut.http.client.annotation.Client;
+import io.micronaut.http.client.exceptions.HttpClientResponseException;
+import io.micronaut.runtime.server.EmbeddedServer;
+import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
+import jakarta.inject.Inject;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.cluster.management.ClusterInitializer;
+import org.apache.ignite.internal.cluster.management.MockNode;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.apache.ignite.network.ClusterService;
+import org.apache.ignite.network.NetworkAddress;
+import org.apache.ignite.network.StaticNodeFinder;
+import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * Cluster management REST test.
+ */
+@MicronautTest
+@ExtendWith(WorkDirectoryExtension.class)
+public class ItClusterManagementControllerTest {
+
+ private static final int PORT_BASE = 10000;
+
+ private static final List<MockNode> cluster = new ArrayList<>();
+ static ClusterService clusterService;
+ @WorkDirectory
+ private static Path workDir;
+ @Inject
+ EmbeddedServer server;
+ @Inject
+ @Client("/management/v1/cluster/init/")
+ HttpClient client;
+ @Inject
+ ClusterInitializer clusterInitializer;
+
+ @BeforeAll
+ static void setUp(TestInfo testInfo) throws IOException {
+ var addr1 = new NetworkAddress("localhost", PORT_BASE);
+ var addr2 = new NetworkAddress("localhost", PORT_BASE + 1);
+
+ var nodeFinder = new StaticNodeFinder(List.of(addr1, addr2));
+
+ cluster.add(new MockNode(testInfo, addr1, nodeFinder, workDir.resolve("node0")));
+ cluster.add(new MockNode(testInfo, addr2, nodeFinder, workDir.resolve("node1")));
+
+ for (MockNode node : cluster) {
+ node.start();
+ }
+
+ clusterService = cluster.get(0).clusterService();
+ }
+
+ @AfterAll
+ static void tearDown() {
+ for (MockNode node : cluster) {
+ node.beforeNodeStop();
+ }
+
+ for (MockNode node : cluster) {
+ node.stop();
+ }
+ }
+
+ @Test
+ void testControllerLoaded() {
+ assertNotNull(server.getApplicationContext().getBean(ClusterManagementController.class));
+ }
+
+ @Test
+ void testInitNoSuchNode() {
+ // Given body with nodename that does not exist
+ String givenInvalidBody = "{\"metaStorageNodes\": [\"nodename\"], \"cmgNodes\": [], \"clusterName\": \"cluster\"}";
+
+ // When
+ var thrown = assertThrows(
+ HttpClientResponseException.class,
+ () -> client.toBlocking().exchange(HttpRequest.POST("", givenInvalidBody))
+ );
+
+ // Then
+ assertThat(thrown.getResponse().getStatus(), is(equalTo((HttpStatus.BAD_REQUEST))));
+ // And
+ var errorResult = getErrorResult(thrown);
+ assertEquals("INVALID_NODES", errorResult.type());
+ }
+
+ @Test
+ void testInitAlreadyInitializedWithAnotherNodes() {
+ // Given cluster initialized
+ String givenFirstRequestBody =
+ "{\"metaStorageNodes\": [\"" + cluster.get(0).clusterService().localConfiguration().getName() + "\"], \"cmgNodes\": [], "
+ + "\"clusterName\": \"cluster\"}";
+
+ // When
+ HttpResponse<Object> response = client.toBlocking().exchange(HttpRequest.POST("", givenFirstRequestBody));
+
+ // Then
+ assertThat(response.getStatus(), is(equalTo((HttpStatus.OK))));
+ // And
+ assertThat(cluster.get(0).startFuture(), willCompleteSuccessfully());
+
+ // Given second request with different node name
+ String givenSecondRequestBody =
+ "{\"metaStorageNodes\": [\"" + cluster.get(1).clusterService().localConfiguration().getName() + "\"], \"cmgNodes\": [], "
+ + "\"clusterName\": \"cluster\" }";
+
+ // When
+ var thrown = assertThrows(
+ HttpClientResponseException.class,
+ () -> client.toBlocking().exchange(HttpRequest.POST("", givenSecondRequestBody))
+ );
+
+ // Then
+ assertThat(thrown.getResponse().getStatus(), is(equalTo((HttpStatus.CONFLICT))));
+ // And
+ var errorResult = getErrorResult(thrown);
+ assertEquals("ALREADY_INITIALIZED", errorResult.type());
+
+ }
+
+ @Factory
+ @Bean
+ @Replaces(ClusterManagementRestFactory.class)
+ public ClusterManagementRestFactory clusterManagementRestFactory() {
+ return new ClusterManagementRestFactory(clusterService);
+ }
+
+ @NotNull
Review Comment:
If I'm not misttaken, we decided to not use `@NotNull` annotation. If something is not annotated with `@Nullable`, it is treated as non-nullable, that's it.
##########
modules/configuration/src/test/java/org/apache/ignite/internal/rest/configuration/ClusterConfigurationControllerTest.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * 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.rest.configuration;
+
+import io.micronaut.context.annotation.Bean;
+import io.micronaut.context.annotation.Replaces;
+import io.micronaut.http.client.HttpClient;
+import io.micronaut.http.client.annotation.Client;
+import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
+import jakarta.inject.Inject;
+import jakarta.inject.Named;
+import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation;
+import org.apache.ignite.internal.configuration.rest.presentation.hocon.HoconPresentation;
+
+/**
+ * Functional test for {@link ClusterConfigurationController}.
+ */
+@MicronautTest
+class ClusterConfigurationControllerTest extends ConfigurationControllerBaseTest {
+
+ @Inject
+ @Client("/management/v1/configuration/cluster/")
+ HttpClient client;
+
+ @Override
+ HttpClient client() {
+ return client;
+ }
+
Review Comment:
An excessive blank line?
##########
modules/configuration/src/main/java/org/apache/ignite/internal/rest/configuration/exception/handler/ConfigPathUnrecognizedExceptionHandler.java:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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.rest.configuration.exception.handler;
+
+import io.micronaut.context.annotation.Requires;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpResponse;
+import io.micronaut.http.MediaType;
+import io.micronaut.http.annotation.Produces;
+import io.micronaut.http.server.exceptions.ExceptionHandler;
+import jakarta.inject.Singleton;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.apache.ignite.internal.rest.configuration.exception.ConfigPathUnrecognizedException;
+
+/**
+ * Handles {@link ConfigPathUnrecognizedException} and represents it as a rest response.
+ */
+@Produces(MediaType.APPLICATION_JSON)
+@Singleton
+@Requires(classes = {ConfigPathUnrecognizedException.class, ExceptionHandler.class})
+public class ConfigPathUnrecognizedExceptionHandler implements
+ ExceptionHandler<ConfigPathUnrecognizedException, HttpResponse<ErrorResult>> {
+
+ @Override
+ public HttpResponse<ErrorResult> handle(HttpRequest request, ConfigPathUnrecognizedException exception) {
+ final ErrorResult errorResult = new ErrorResult("CONFIG_PATH_UNRECOGNIZED", exception.getMessage());
Review Comment:
`final` seems redundant here as the method is just 2 lines
##########
modules/configuration/src/main/java/org/apache/ignite/internal/rest/configuration/AbstractConfigurationController.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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.rest.configuration;
+
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionException;
+import org.apache.ignite.configuration.validation.ConfigurationValidationException;
+import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation;
+import org.apache.ignite.internal.rest.configuration.exception.ConfigPathUnrecognizedException;
+import org.apache.ignite.internal.rest.configuration.exception.InvalidConfigFormatException;
+import org.apache.ignite.lang.IgniteException;
+
+/**
+ * Base configuration controller.
+ */
+public class AbstractConfigurationController {
Review Comment:
I suggest making this class `abstract`, as its name implies
##########
modules/cli/src/main/java/org/apache/ignite/cli/builtins/config/ConfigurationClient.java:
##########
@@ -81,7 +81,7 @@ public String get(
) {
var req = HttpRequest
.newBuilder()
- .header("Content-Type", "application/json");
+ .header("Content-Type", "text/plain");
Review Comment:
It's not a JSON, but it's not just *any* plain text. Could we use a specific content type here? For example, here https://www.w3schools.io/file/hocon-introduction/ they suggest `application/hocon`; it does not seem to be registered with IANA, but it looks logical.
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/ClusterManagementController.java:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.cluster.management.rest;
+
+import io.micronaut.http.MediaType;
+import io.micronaut.http.annotation.Body;
+import io.micronaut.http.annotation.Consumes;
+import io.micronaut.http.annotation.Controller;
+import io.micronaut.http.annotation.Post;
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import java.util.concurrent.ExecutionException;
+import org.apache.ignite.internal.cluster.management.ClusterInitializer;
+import org.apache.ignite.internal.cluster.management.rest.exception.InvalidNodesInitClusterException;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.IgniteInternalException;
+import org.apache.ignite.lang.IgniteLogger;
+
+/**
+ * Cluster management controller.
+ */
+@Controller("/management/v1/cluster/init")
+@ApiResponse(responseCode = "400", description = "Incorrect body")
Review Comment:
`400` could potentially mean that something else is wrong with the request, not just its body. Should we be so specific here? Would 'Bad request' or 'Invalid request' be a safer alternative?
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java:
##########
@@ -17,151 +17,147 @@
package org.apache.ignite.internal.rest;
-import io.netty.bootstrap.ServerBootstrap;
-import io.netty.channel.Channel;
-import io.netty.channel.ChannelFuture;
-import io.netty.handler.logging.LogLevel;
-import io.netty.handler.logging.LoggingHandler;
+import io.micronaut.context.ApplicationContext;
+import io.micronaut.http.server.exceptions.ServerStartupException;
+import io.micronaut.openapi.annotation.OpenAPIInclude;
+import io.micronaut.runtime.Micronaut;
+import io.micronaut.runtime.exceptions.ApplicationStartupException;
+import io.swagger.v3.oas.annotations.OpenAPIDefinition;
+import io.swagger.v3.oas.annotations.info.Contact;
+import io.swagger.v3.oas.annotations.info.Info;
+import io.swagger.v3.oas.annotations.info.License;
import java.net.BindException;
-import java.net.InetSocketAddress;
-import java.util.function.Consumer;
+import java.util.List;
+import java.util.Map;
import org.apache.ignite.configuration.schemas.rest.RestConfiguration;
import org.apache.ignite.configuration.schemas.rest.RestView;
-import org.apache.ignite.internal.configuration.ConfigurationManager;
-import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController;
import org.apache.ignite.internal.manager.IgniteComponent;
-import org.apache.ignite.internal.rest.api.RestHandlersRegister;
-import org.apache.ignite.internal.rest.api.Routes;
-import org.apache.ignite.internal.rest.netty.RestApiInitializer;
-import org.apache.ignite.internal.rest.routes.Router;
-import org.apache.ignite.internal.rest.routes.SimpleRouter;
-import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.internal.rest.api.RestFactory;
+import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController;
+import org.apache.ignite.internal.rest.configuration.NodeConfigurationController;
import org.apache.ignite.lang.IgniteInternalException;
import org.apache.ignite.lang.IgniteLogger;
-import org.apache.ignite.network.NettyBootstrapFactory;
/**
- * Rest component is responsible for starting REST endpoints.
+ * Rest module is responsible for starting a REST endpoints for accessing and managing configuration.
*
- * <p>It is started on port 10300 by default, but it is possible to change this in configuration itself. Refer to default config file in
+ * <p>It is started on port 10300 by default but it is possible to change this in configuration itself. Refer to default config file in
* resources for the example.
*/
-public class RestComponent implements RestHandlersRegister, IgniteComponent {
+@OpenAPIDefinition(info = @Info(title = "Ignite REST module", version = "3.0.0-alpha", license = @License(name = "Apache 2.0", url = "https://ignite.apache.org"), contact = @Contact(email = "user@ignite.apache.org")))
+@OpenAPIInclude(classes = {ClusterConfigurationController.class, NodeConfigurationController.class, ClusterManagementController.class})
+public class RestComponent implements IgniteComponent {
+ /** Default port. */
+ public static final int DFLT_PORT = 10300;
+
/** Ignite logger. */
private final IgniteLogger log = IgniteLogger.forClass(RestComponent.class);
- /** Node configuration register. */
- private final ConfigurationRegistry nodeCfgRegistry;
-
- /** Netty bootstrap factory. */
- private final NettyBootstrapFactory bootstrapFactory;
-
- private final SimpleRouter router = new SimpleRouter();
-
- /** Netty channel. */
- private volatile Channel channel;
+ /** Factories that produce beans needed for REST controllers. */
+ private final List<RestFactory> restFactories;
+ private final RestConfiguration restConfiguration;
+ /** Server host. */
+ private final String host = "localhost";
+ /** Micronaut application context. */
+ private ApplicationContext context;
+ /** Server port. */
+ private int port;
/**
* Creates a new instance of REST module.
- *
- * @param nodeCfgMgr Node configuration manager.
- * @param bootstrapFactory Netty bootstrap factory.
*/
- public RestComponent(ConfigurationManager nodeCfgMgr, NettyBootstrapFactory bootstrapFactory) {
- nodeCfgRegistry = nodeCfgMgr.configurationRegistry();
-
- this.bootstrapFactory = bootstrapFactory;
- }
-
- /** {@inheritDoc} */
- @Override
- public void registerHandlers(Consumer<Routes> registerAction) {
- registerAction.accept(router);
+ public RestComponent(List<RestFactory> restFactories, RestConfiguration restConfiguration) {
+ this.restFactories = restFactories;
+ this.restConfiguration = restConfiguration;
}
/** {@inheritDoc} */
@Override
public void start() {
- if (channel != null) {
- throw new IgniteException("RestModule is already started.");
- }
-
- channel = startRestEndpoint(router).channel();
- }
-
- /**
- * Start endpoint.
- *
- * @param router Dispatcher of http requests.
- * @return Future which will be notified when this channel is closed.
- * @throws RuntimeException if this module cannot be bound to a port.
- */
- private ChannelFuture startRestEndpoint(Router router) {
- RestView restConfigurationView = nodeCfgRegistry.getConfiguration(RestConfiguration.KEY).value();
+ RestView restConfigurationView = restConfiguration.value();
int desiredPort = restConfigurationView.port();
int portRange = restConfigurationView.portRange();
- int port = 0;
- Channel ch = null;
-
- ServerBootstrap bootstrap = bootstrapFactory.createServerBootstrap()
- .handler(new LoggingHandler(LogLevel.INFO))
- .childHandler(new RestApiInitializer(router));
-
for (int portCandidate = desiredPort; portCandidate <= desiredPort + portRange; portCandidate++) {
- ChannelFuture bindRes = bootstrap.bind(portCandidate).awaitUninterruptibly();
-
- if (bindRes.isSuccess()) {
- ch = bindRes.channel();
+ try {
+ context = buildMicronautContext(portCandidate).start();
port = portCandidate;
+ log.info("REST protocol started successfully");
break;
- } else if (!(bindRes.cause() instanceof BindException)) {
- throw new RuntimeException(bindRes.cause());
+ } catch (ApplicationStartupException e) {
+ log.error("Got exception " + e.getCause() + " during node start on port " + portCandidate + " , trying again");
}
}
- if (ch == null) {
- String msg = "Cannot start REST endpoint. "
- + "All ports in range [" + desiredPort + ", " + (desiredPort + portRange) + "] are in use.";
+ if (context == null) {
+ String msg = "Cannot start REST endpoint. " + "All ports in range [" + desiredPort + ", " + (desiredPort + portRange)
+ + "] are in use.";
log.error(msg);
throw new RuntimeException(msg);
}
+ }
+
+ private Micronaut buildMicronautContext(int portCandidate) {
+ Micronaut micronaut = Micronaut.build("");
+ setFactories(micronaut);
+ return micronaut
+ .properties(Map.of("micronaut.server.port", portCandidate))
+ .banner(false)
+ .mapError(ServerStartupException.class, this::mapServerStartupException)
+ .mapError(ApplicationStartupException.class, ex -> -1);
+ }
- if (log.isInfoEnabled()) {
- log.info("REST protocol started successfully on port " + port);
+ private void setFactories(Micronaut micronaut) {
+ for (var factory : restFactories) {
+ micronaut.singletons(factory);
}
+ }
- return ch.closeFuture();
+ private int mapServerStartupException(ServerStartupException exception) {
+ if (exception.getCause() instanceof BindException) {
+ return -1;
Review Comment:
What do these 1 and -1 mean in this case?
##########
modules/cli/src/integrationTest/java/org/apache/ignite/cli/ItConfigCommandTest.java:
##########
@@ -113,9 +109,7 @@ public void setAndGetWithManualHost() {
assertEquals(0, exitCode);
- DocumentContext document = JsonPath.parse(removeTrailingQuotes(unescapeQuotes(out.toString(UTF_8))));
-
- assertEquals(1, document.read("$.network.shutdownQuietPeriod", Integer.class));
+ assertTrue(out.toString(UTF_8).contains("\"shutdownQuietPeriod\" : 1"));
Review Comment:
Also, the test before the modification makes less checks on the returned value. The old test seemed to check (implicitly) that the returned value was a valid JSON; also, it looks like now the test is a bit fragile because it will fail if whitespace policy changes.
Is this caused by changing the content type from application/json to text/plain? As we know that this is not just a plain text, but a HOCON document, would it be possible to restore the parsing to make sure it's a valid HOCON?
##########
modules/configuration/src/main/java/org/apache/ignite/internal/rest/configuration/ClusterConfigurationController.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.rest.configuration;
+
+import io.micronaut.http.MediaType;
+import io.micronaut.http.annotation.Body;
+import io.micronaut.http.annotation.Consumes;
+import io.micronaut.http.annotation.Controller;
+import io.micronaut.http.annotation.Get;
+import io.micronaut.http.annotation.Patch;
+import io.micronaut.http.annotation.PathVariable;
+import io.micronaut.http.annotation.Produces;
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.media.Content;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import jakarta.inject.Named;
+import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation;
+
+/**
+ * Cluster configuration controller.
+ */
+@Controller("/management/v1/configuration/cluster/")
+@ApiResponse(responseCode = "400", description = "Incorrect configuration")
+@ApiResponse(responseCode = "500", description = "Internal error")
+@Tag(name = "clusterConfiguration")
+public class ClusterConfigurationController extends AbstractConfigurationController {
+
+ public ClusterConfigurationController(@Named("clusterCfgPresentation") ConfigurationPresentation<String> clusterCfgPresentation) {
+ super(clusterCfgPresentation);
+ }
+
+ /**
+ * Returns cluster configuration in HOCON format.
+ *
+ * @return the whole cluster configuration in HOCON format.
+ */
+ @Operation(operationId = "getClusterConfiguration")
+ @ApiResponse(
+ responseCode = "200",
+ content = @Content(mediaType = MediaType.TEXT_PLAIN, schema = @Schema(type = "string")),
+ description = "Get cluster configuration")
+ @Produces(MediaType.TEXT_PLAIN)
Review Comment:
`application/hocon`?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r885084300
##########
modules/rest-api/src/main/java/org/apache/ignite/internal/rest/api/RestFactory.java:
##########
@@ -15,8 +15,10 @@
* limitations under the License.
*/
+package org.apache.ignite.internal.rest.api;
+
/**
- * This package contains classes that represent inbound requests, outbound responses and main REST handler.
+ * Factory that produces all beans that is necessary for the controller class.
*/
-
-package org.apache.ignite.internal.rest.netty;
+public interface RestFactory {
Review Comment:
This interface represents the factory that is needed for a controller, that is defined in a module. RestComponent takes the list of this factories and puts to the micronaut context.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r885551636
##########
modules/cluster-management/pom.xml:
##########
@@ -49,17 +49,26 @@
<dependency>
<groupId>org.apache.ignite</groupId>
- <artifactId>ignite-rest</artifactId>
+ <artifactId>ignite-vault</artifactId>
</dependency>
<dependency>
<groupId>org.apache.ignite</groupId>
- <artifactId>ignite-vault</artifactId>
+ <artifactId>ignite-rest-api</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>io.micronaut</groupId>
+ <artifactId>micronaut-http</artifactId>
+ <version>${micronaut.version}</version>
Review Comment:
> As for micronaut-bom, it might not pass vulnerabilities checks
Why? There should be no difference between using a bom and declaring dependencies directly
> That could be a problem in dependency management.
Can you provide an example please?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r886919324
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java:
##########
@@ -17,151 +17,147 @@
package org.apache.ignite.internal.rest;
-import io.netty.bootstrap.ServerBootstrap;
-import io.netty.channel.Channel;
-import io.netty.channel.ChannelFuture;
-import io.netty.handler.logging.LogLevel;
-import io.netty.handler.logging.LoggingHandler;
+import io.micronaut.context.ApplicationContext;
+import io.micronaut.http.server.exceptions.ServerStartupException;
+import io.micronaut.openapi.annotation.OpenAPIInclude;
+import io.micronaut.runtime.Micronaut;
+import io.micronaut.runtime.exceptions.ApplicationStartupException;
+import io.swagger.v3.oas.annotations.OpenAPIDefinition;
+import io.swagger.v3.oas.annotations.info.Contact;
+import io.swagger.v3.oas.annotations.info.Info;
+import io.swagger.v3.oas.annotations.info.License;
import java.net.BindException;
-import java.net.InetSocketAddress;
-import java.util.function.Consumer;
+import java.util.List;
+import java.util.Map;
import org.apache.ignite.configuration.schemas.rest.RestConfiguration;
import org.apache.ignite.configuration.schemas.rest.RestView;
-import org.apache.ignite.internal.configuration.ConfigurationManager;
-import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController;
import org.apache.ignite.internal.manager.IgniteComponent;
-import org.apache.ignite.internal.rest.api.RestHandlersRegister;
-import org.apache.ignite.internal.rest.api.Routes;
-import org.apache.ignite.internal.rest.netty.RestApiInitializer;
-import org.apache.ignite.internal.rest.routes.Router;
-import org.apache.ignite.internal.rest.routes.SimpleRouter;
-import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.internal.rest.api.RestFactory;
+import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController;
+import org.apache.ignite.internal.rest.configuration.NodeConfigurationController;
import org.apache.ignite.lang.IgniteInternalException;
import org.apache.ignite.lang.IgniteLogger;
-import org.apache.ignite.network.NettyBootstrapFactory;
/**
- * Rest component is responsible for starting REST endpoints.
+ * Rest module is responsible for starting a REST endpoints for accessing and managing configuration.
*
- * <p>It is started on port 10300 by default, but it is possible to change this in configuration itself. Refer to default config file in
+ * <p>It is started on port 10300 by default but it is possible to change this in configuration itself. Refer to default config file in
* resources for the example.
*/
-public class RestComponent implements RestHandlersRegister, IgniteComponent {
+@OpenAPIDefinition(info = @Info(title = "Ignite REST module", version = "3.0.0-alpha", license = @License(name = "Apache 2.0", url = "https://ignite.apache.org"), contact = @Contact(email = "user@ignite.apache.org")))
+@OpenAPIInclude(classes = {ClusterConfigurationController.class, NodeConfigurationController.class, ClusterManagementController.class})
+public class RestComponent implements IgniteComponent {
+ /** Default port. */
+ public static final int DFLT_PORT = 10300;
+
/** Ignite logger. */
private final IgniteLogger log = IgniteLogger.forClass(RestComponent.class);
- /** Node configuration register. */
- private final ConfigurationRegistry nodeCfgRegistry;
-
- /** Netty bootstrap factory. */
- private final NettyBootstrapFactory bootstrapFactory;
-
- private final SimpleRouter router = new SimpleRouter();
-
- /** Netty channel. */
- private volatile Channel channel;
+ /** Factories that produce beans needed for REST controllers. */
+ private final List<RestFactory> restFactories;
+ private final RestConfiguration restConfiguration;
+ /** Server host. */
+ private final String host = "localhost";
+ /** Micronaut application context. */
+ private ApplicationContext context;
+ /** Server port. */
+ private int port;
/**
* Creates a new instance of REST module.
- *
- * @param nodeCfgMgr Node configuration manager.
- * @param bootstrapFactory Netty bootstrap factory.
*/
- public RestComponent(ConfigurationManager nodeCfgMgr, NettyBootstrapFactory bootstrapFactory) {
- nodeCfgRegistry = nodeCfgMgr.configurationRegistry();
-
- this.bootstrapFactory = bootstrapFactory;
- }
-
- /** {@inheritDoc} */
- @Override
- public void registerHandlers(Consumer<Routes> registerAction) {
- registerAction.accept(router);
+ public RestComponent(List<RestFactory> restFactories, RestConfiguration restConfiguration) {
+ this.restFactories = restFactories;
+ this.restConfiguration = restConfiguration;
}
/** {@inheritDoc} */
@Override
public void start() {
- if (channel != null) {
- throw new IgniteException("RestModule is already started.");
- }
-
- channel = startRestEndpoint(router).channel();
- }
-
- /**
- * Start endpoint.
- *
- * @param router Dispatcher of http requests.
- * @return Future which will be notified when this channel is closed.
- * @throws RuntimeException if this module cannot be bound to a port.
- */
- private ChannelFuture startRestEndpoint(Router router) {
- RestView restConfigurationView = nodeCfgRegistry.getConfiguration(RestConfiguration.KEY).value();
+ RestView restConfigurationView = restConfiguration.value();
int desiredPort = restConfigurationView.port();
int portRange = restConfigurationView.portRange();
- int port = 0;
- Channel ch = null;
-
- ServerBootstrap bootstrap = bootstrapFactory.createServerBootstrap()
- .handler(new LoggingHandler(LogLevel.INFO))
- .childHandler(new RestApiInitializer(router));
-
for (int portCandidate = desiredPort; portCandidate <= desiredPort + portRange; portCandidate++) {
- ChannelFuture bindRes = bootstrap.bind(portCandidate).awaitUninterruptibly();
-
- if (bindRes.isSuccess()) {
- ch = bindRes.channel();
+ try {
+ context = buildMicronautContext(portCandidate).start();
port = portCandidate;
+ log.info("REST protocol started successfully");
break;
- } else if (!(bindRes.cause() instanceof BindException)) {
- throw new RuntimeException(bindRes.cause());
+ } catch (ApplicationStartupException e) {
+ log.error("Got exception " + e.getCause() + " during node start on port " + portCandidate + " , trying again");
Review Comment:
Yes, this is strange. I'll rewrite it.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r886899504
##########
modules/rest/pom.xml:
##########
@@ -151,7 +128,21 @@
<artifactId>ignite-configuration-annotation-processor</artifactId>
<version>${project.version}</version>
</path>
+ <path>
+ <groupId>io.micronaut</groupId>
+ <artifactId>micronaut-inject-java</artifactId>
+ <version>${micronaut.version}</version>
+ </path>
+ <path>
+ <groupId>io.micronaut.openapi</groupId>
+ <artifactId>micronaut-openapi</artifactId>
+ <version>${maven.micronaut.openapi.plugin.version}</version>
+ </path>
</annotationProcessorPaths>
+ <fork>true</fork>
Review Comment:
Runnin the compiler in separate thread but I'll remove it in order not to confuse engineers.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r886875205
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/ClusterManagementController.java:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.cluster.management.rest;
+
+import io.micronaut.http.MediaType;
+import io.micronaut.http.annotation.Body;
+import io.micronaut.http.annotation.Consumes;
+import io.micronaut.http.annotation.Controller;
+import io.micronaut.http.annotation.Post;
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import java.util.concurrent.ExecutionException;
+import org.apache.ignite.internal.cluster.management.ClusterInitializer;
+import org.apache.ignite.internal.cluster.management.rest.exception.InvalidNodesInitClusterException;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.IgniteInternalException;
+import org.apache.ignite.lang.IgniteLogger;
+
+/**
+ * Cluster management controller.
+ */
+@Controller("/management/v1/cluster/init")
+@ApiResponse(responseCode = "400", description = "Incorrect body")
+@ApiResponse(responseCode = "500", description = "Internal error")
+@Tag(name = "clusterManagement")
+public class ClusterManagementController {
+ private static final IgniteLogger log = IgniteLogger.forClass(ClusterManagementController.class);
+
+ private final ClusterInitializer clusterInitializer;
+
+ /**
+ * Cluster management controller constructor.
+ *
+ * @param clusterInitializer cluster initializer.
+ */
+ public ClusterManagementController(ClusterInitializer clusterInitializer) {
+ this.clusterInitializer = clusterInitializer;
+ }
+
+ /**
+ * Initializes cluster.
+ */
+ @Post
+ @Operation(operationId = "init")
+ @ApiResponse(responseCode = "200", description = "Cluster initialized")
+ @ApiResponse(responseCode = "409", description = "Cluster already initialized")
+ @Consumes(MediaType.APPLICATION_JSON)
+ public void init(@Body InitCommand initCommand) throws ExecutionException, InterruptedException {
+ if (log.isInfoEnabled()) {
+ log.info("Received init command:\n\tMeta Storage nodes: {}\n\tCMG nodes: {}", initCommand.metaStorageNodes(),
+ initCommand.cmgNodes());
+ }
+
+ try {
+ clusterInitializer.initCluster(initCommand.metaStorageNodes(), initCommand.cmgNodes(), initCommand.clusterName()).get();
+ } catch (ExecutionException ex) {
+ Throwable cause = ex.getCause();
+ handleThrowable(cause);
+ }
+ }
+
+ private void handleThrowable(Throwable cause) {
+ if (cause instanceof IgniteInternalException) {
+ throw (IgniteInternalException) cause;
+ } else if (cause instanceof IllegalArgumentException) {
+ throw new InvalidNodesInitClusterException(cause);
Review Comment:
Thanks, I will name it in another way.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r885082677
##########
modules/cluster-management/src/integrationTest/java/org/apache/ignite/internal/cluster/management/ItClusterManagerTest.java:
##########
@@ -107,13 +107,13 @@ void testInit(TestInfo testInfo) throws Exception {
initCluster(metaStorageNodes, cmgNodes);
- assertThat(cluster.get(0).clusterManager().metaStorageNodes(), will(containsInAnyOrder(metaStorageNodes)));
- assertThat(cluster.get(1).clusterManager().metaStorageNodes(), will(containsInAnyOrder(metaStorageNodes)));
+ assertThat(cluster.get(0).getClusterManager().metaStorageNodes(), will(containsInAnyOrder(metaStorageNodes)));
+ assertThat(cluster.get(1).getClusterManager().metaStorageNodes(), will(containsInAnyOrder(metaStorageNodes)));
Review Comment:
Thanks for the note, I think this was a mistake. I'll revert this back.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r885083749
##########
modules/cli/src/main/java/org/apache/ignite/cli/builtins/config/ConfigurationClient.java:
##########
@@ -81,7 +81,7 @@ public String get(
) {
var req = HttpRequest
.newBuilder()
- .header("Content-Type", "application/json");
+ .header("Content-Type", "text/plain");
Review Comment:
I've changed the Content-Type from applicaiton/json to text/plain because the HOCON configuration, in fact, is represented as a text (string) but not as a json object.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r885593591
##########
modules/cluster-management/pom.xml:
##########
@@ -49,17 +49,26 @@
<dependency>
<groupId>org.apache.ignite</groupId>
- <artifactId>ignite-rest</artifactId>
+ <artifactId>ignite-vault</artifactId>
</dependency>
<dependency>
<groupId>org.apache.ignite</groupId>
- <artifactId>ignite-vault</artifactId>
+ <artifactId>ignite-rest-api</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>io.micronaut</groupId>
+ <artifactId>micronaut-http</artifactId>
+ <version>${micronaut.version}</version>
Review Comment:
> Why? There should be no difference between using a bom and declaring dependencies directly
Yes, you are right, there is no difference.
> Can you provide an example please?
Micronaut-bom includes a lot of other libraries' version definitions, for example, neo4j.version, h2.version, and many more. So, Ignite 3 will have them included to dependency management too. Maybe that is not a problem but if Iginite 3 would like to support neo4j with a version that is different from micronaut-bom then we might face an issue.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r885491441
##########
modules/rest-client/pom.xml:
##########
@@ -0,0 +1,124 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ 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.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <parent>
+ <groupId>org.apache.ignite</groupId>
+ <artifactId>ignite-parent</artifactId>
+ <version>1</version>
+ <relativePath>../../parent/pom.xml</relativePath>
+ </parent>
+
+ <artifactId>ignite-rest-client</artifactId>
+ <version>3.0.0-SNAPSHOT</version>
+
+ <dependencies>
+
+ <!--client specific dependencies, versions cannot be moved to the parent pom-->
Review Comment:
> drop this module and create a ticket for integration tests that will generate the client somewhere inside and not include it as a module.
That would be very much preferable, thank you
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r886888985
##########
modules/configuration/src/test/java/org/apache/ignite/internal/rest/configuration/ConfigurationControllerBaseTest.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.ignite.internal.rest.configuration;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import io.micronaut.context.ApplicationContext;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpStatus;
+import io.micronaut.http.MediaType;
+import io.micronaut.http.client.HttpClient;
+import io.micronaut.http.client.exceptions.HttpClientResponseException;
+import io.micronaut.runtime.server.EmbeddedServer;
+import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
+import jakarta.inject.Inject;
+import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation;
+import org.apache.ignite.internal.configuration.rest.presentation.TestRootConfiguration;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * The base test for configuration controllers.
+ */
+@MicronautTest
+public abstract class ConfigurationControllerBaseTest {
+ @Inject
+ EmbeddedServer server;
Review Comment:
Yes
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r886886673
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalExceptionHandler.java:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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.cluster.management.rest.exception.handler;
+
+import io.micronaut.context.annotation.Requires;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpResponse;
+import io.micronaut.http.HttpResponseFactory;
+import io.micronaut.http.HttpStatus;
+import io.micronaut.http.annotation.Produces;
+import io.micronaut.http.server.exceptions.ExceptionHandler;
+import jakarta.inject.Singleton;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.apache.ignite.lang.IgniteInternalException;
+
+/**
+ * Handles {@link IgniteInternalException} and represents it as a rest response.
+ */
+@Produces
+@Singleton
+@Requires(classes = {IgniteInternalException.class, ExceptionHandler.class})
+public class IgniteInternalExceptionHandler implements ExceptionHandler<IgniteInternalException, HttpResponse<ErrorResult>> {
+
+ @Override
+ public HttpResponse<ErrorResult> handle(HttpRequest request, IgniteInternalException exception) {
+ final ErrorResult errorResult = new ErrorResult("ALREADY_INITIALIZED", exception.getMessage());
+ return HttpResponseFactory.INSTANCE.status(HttpStatus.CONFLICT).body(errorResult);
Review Comment:
Since I've named it INITIALIZATION_EXCEPTION now this question is out of date.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r887830283
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalExceptionHandler.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * 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.cluster.management.rest.exception.handler;
+
+import io.micronaut.context.annotation.Requires;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpResponse;
+import io.micronaut.http.annotation.Produces;
+import io.micronaut.http.server.exceptions.ExceptionHandler;
+import jakarta.inject.Singleton;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.apache.ignite.lang.IgniteInternalException;
+
+/**
+ * Handles {@link IgniteInternalException} and represents it as a rest response.
+ */
+@Produces
+@Singleton
+@Requires(classes = {IgniteInternalException.class, ExceptionHandler.class})
+public class IgniteInternalExceptionHandler implements ExceptionHandler<IgniteInternalException, HttpResponse<ErrorResult>> {
+
+ @Override
+ public HttpResponse<ErrorResult> handle(HttpRequest request, IgniteInternalException exception) {
+ ErrorResult errorResult = new ErrorResult("INITIALIZATION_EXCEPTION", exception.getMessage());
+ return HttpResponse.badRequest().body(errorResult);
Review Comment:
> IgniteInternalException looks like 500 and I think it is better to mark it as an internal server error.
Exactly
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r887333292
##########
modules/cli/src/integrationTest/java/org/apache/ignite/cli/ItConfigCommandTest.java:
##########
@@ -113,9 +109,7 @@ public void setAndGetWithManualHost() {
assertEquals(0, exitCode);
- DocumentContext document = JsonPath.parse(removeTrailingQuotes(unescapeQuotes(out.toString(UTF_8))));
-
- assertEquals(1, document.read("$.network.shutdownQuietPeriod", Integer.class));
+ assertTrue(out.toString(UTF_8).contains("\"shutdownQuietPeriod\" : 1"));
Review Comment:
> Is this caused by changing the content type from application/json to text/plain?
Yes
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r885884267
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/ClusterManagementController.java:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.cluster.management.rest;
+
+import io.micronaut.http.MediaType;
+import io.micronaut.http.annotation.Body;
+import io.micronaut.http.annotation.Consumes;
+import io.micronaut.http.annotation.Controller;
+import io.micronaut.http.annotation.Post;
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import java.util.concurrent.ExecutionException;
+import org.apache.ignite.internal.cluster.management.ClusterInitializer;
+import org.apache.ignite.internal.cluster.management.rest.exception.InvalidNodesInitClusterException;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.IgniteInternalException;
+import org.apache.ignite.lang.IgniteLogger;
+
+/**
+ * Cluster management controller.
+ */
+@Controller("/management/v1/cluster/init")
+@ApiResponse(responseCode = "400", description = "Incorrect body")
+@ApiResponse(responseCode = "500", description = "Internal error")
+@Tag(name = "clusterManagement")
+public class ClusterManagementController {
+ private static final IgniteLogger log = IgniteLogger.forClass(ClusterManagementController.class);
+
+ private final ClusterInitializer clusterInitializer;
+
+ /**
+ * Cluster management controller constructor.
+ *
+ * @param clusterInitializer cluster initializer.
+ */
+ public ClusterManagementController(ClusterInitializer clusterInitializer) {
+ this.clusterInitializer = clusterInitializer;
+ }
+
+ /**
+ * Initializes cluster.
+ */
+ @Post
+ @Operation(operationId = "init")
+ @ApiResponse(responseCode = "200", description = "Cluster initialized")
+ @ApiResponse(responseCode = "409", description = "Cluster already initialized")
+ @Consumes(MediaType.APPLICATION_JSON)
+ public void init(@Body InitCommand initCommand) throws ExecutionException, InterruptedException {
Review Comment:
Same here: I think it should be possible to return a CompletableFuture directly
##########
modules/cluster-management/src/integrationTest/java/org/apache/ignite/internal/cluster/management/rest/ItClusterManagementControllerTest.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * 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.cluster.management.rest;
+
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import io.micronaut.context.annotation.Bean;
+import io.micronaut.context.annotation.Factory;
+import io.micronaut.context.annotation.Replaces;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpResponse;
+import io.micronaut.http.HttpStatus;
+import io.micronaut.http.client.HttpClient;
+import io.micronaut.http.client.annotation.Client;
+import io.micronaut.http.client.exceptions.HttpClientResponseException;
+import io.micronaut.runtime.server.EmbeddedServer;
+import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
+import jakarta.inject.Inject;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.cluster.management.ClusterInitializer;
+import org.apache.ignite.internal.cluster.management.MockNode;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.apache.ignite.network.ClusterService;
+import org.apache.ignite.network.NetworkAddress;
+import org.apache.ignite.network.StaticNodeFinder;
+import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * Cluster management REST test.
+ */
+@MicronautTest
+@ExtendWith(WorkDirectoryExtension.class)
+public class ItClusterManagementControllerTest {
+
+ private static final int PORT_BASE = 10000;
+
+ private static final List<MockNode> cluster = new ArrayList<>();
Review Comment:
please separate the fields with line breaks
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/ClusterManagementController.java:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.cluster.management.rest;
+
+import io.micronaut.http.MediaType;
+import io.micronaut.http.annotation.Body;
+import io.micronaut.http.annotation.Consumes;
+import io.micronaut.http.annotation.Controller;
+import io.micronaut.http.annotation.Post;
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import java.util.concurrent.ExecutionException;
+import org.apache.ignite.internal.cluster.management.ClusterInitializer;
+import org.apache.ignite.internal.cluster.management.rest.exception.InvalidNodesInitClusterException;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.IgniteInternalException;
+import org.apache.ignite.lang.IgniteLogger;
+
+/**
+ * Cluster management controller.
+ */
+@Controller("/management/v1/cluster/init")
+@ApiResponse(responseCode = "400", description = "Incorrect body")
+@ApiResponse(responseCode = "500", description = "Internal error")
+@Tag(name = "clusterManagement")
+public class ClusterManagementController {
+ private static final IgniteLogger log = IgniteLogger.forClass(ClusterManagementController.class);
+
+ private final ClusterInitializer clusterInitializer;
+
+ /**
+ * Cluster management controller constructor.
+ *
+ * @param clusterInitializer cluster initializer.
+ */
+ public ClusterManagementController(ClusterInitializer clusterInitializer) {
+ this.clusterInitializer = clusterInitializer;
+ }
+
+ /**
+ * Initializes cluster.
+ */
+ @Post
+ @Operation(operationId = "init")
+ @ApiResponse(responseCode = "200", description = "Cluster initialized")
+ @ApiResponse(responseCode = "409", description = "Cluster already initialized")
+ @Consumes(MediaType.APPLICATION_JSON)
+ public void init(@Body InitCommand initCommand) throws ExecutionException, InterruptedException {
+ if (log.isInfoEnabled()) {
+ log.info("Received init command:\n\tMeta Storage nodes: {}\n\tCMG nodes: {}", initCommand.metaStorageNodes(),
+ initCommand.cmgNodes());
+ }
+
+ try {
+ clusterInitializer.initCluster(initCommand.metaStorageNodes(), initCommand.cmgNodes(), initCommand.clusterName()).get();
+ } catch (ExecutionException ex) {
+ Throwable cause = ex.getCause();
+ handleThrowable(cause);
+ }
+ }
+
+ private void handleThrowable(Throwable cause) {
+ if (cause instanceof IgniteInternalException) {
+ throw (IgniteInternalException) cause;
+ } else if (cause instanceof IllegalArgumentException) {
+ throw new InvalidNodesInitClusterException(cause);
Review Comment:
I'm not sure that it is safe to say that `IllegalArgumentException` is always related to "invalid nodes"
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalExceptionHandler.java:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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.cluster.management.rest.exception.handler;
+
+import io.micronaut.context.annotation.Requires;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpResponse;
+import io.micronaut.http.HttpResponseFactory;
+import io.micronaut.http.HttpStatus;
+import io.micronaut.http.annotation.Produces;
+import io.micronaut.http.server.exceptions.ExceptionHandler;
+import jakarta.inject.Singleton;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.apache.ignite.lang.IgniteInternalException;
+
+/**
+ * Handles {@link IgniteInternalException} and represents it as a rest response.
+ */
+@Produces
+@Singleton
+@Requires(classes = {IgniteInternalException.class, ExceptionHandler.class})
+public class IgniteInternalExceptionHandler implements ExceptionHandler<IgniteInternalException, HttpResponse<ErrorResult>> {
+
+ @Override
+ public HttpResponse<ErrorResult> handle(HttpRequest request, IgniteInternalException exception) {
+ final ErrorResult errorResult = new ErrorResult("ALREADY_INITIALIZED", exception.getMessage());
Review Comment:
Again, I don't think that all `IgniteInternalException`s should be described as `ALREADY_INITIALIZED`
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java:
##########
@@ -17,151 +17,147 @@
package org.apache.ignite.internal.rest;
-import io.netty.bootstrap.ServerBootstrap;
-import io.netty.channel.Channel;
-import io.netty.channel.ChannelFuture;
-import io.netty.handler.logging.LogLevel;
-import io.netty.handler.logging.LoggingHandler;
+import io.micronaut.context.ApplicationContext;
+import io.micronaut.http.server.exceptions.ServerStartupException;
+import io.micronaut.openapi.annotation.OpenAPIInclude;
+import io.micronaut.runtime.Micronaut;
+import io.micronaut.runtime.exceptions.ApplicationStartupException;
+import io.swagger.v3.oas.annotations.OpenAPIDefinition;
+import io.swagger.v3.oas.annotations.info.Contact;
+import io.swagger.v3.oas.annotations.info.Info;
+import io.swagger.v3.oas.annotations.info.License;
import java.net.BindException;
-import java.net.InetSocketAddress;
-import java.util.function.Consumer;
+import java.util.List;
+import java.util.Map;
import org.apache.ignite.configuration.schemas.rest.RestConfiguration;
import org.apache.ignite.configuration.schemas.rest.RestView;
-import org.apache.ignite.internal.configuration.ConfigurationManager;
-import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController;
import org.apache.ignite.internal.manager.IgniteComponent;
-import org.apache.ignite.internal.rest.api.RestHandlersRegister;
-import org.apache.ignite.internal.rest.api.Routes;
-import org.apache.ignite.internal.rest.netty.RestApiInitializer;
-import org.apache.ignite.internal.rest.routes.Router;
-import org.apache.ignite.internal.rest.routes.SimpleRouter;
-import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.internal.rest.api.RestFactory;
+import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController;
+import org.apache.ignite.internal.rest.configuration.NodeConfigurationController;
import org.apache.ignite.lang.IgniteInternalException;
import org.apache.ignite.lang.IgniteLogger;
-import org.apache.ignite.network.NettyBootstrapFactory;
/**
- * Rest component is responsible for starting REST endpoints.
+ * Rest module is responsible for starting a REST endpoints for accessing and managing configuration.
*
- * <p>It is started on port 10300 by default, but it is possible to change this in configuration itself. Refer to default config file in
+ * <p>It is started on port 10300 by default but it is possible to change this in configuration itself. Refer to default config file in
* resources for the example.
*/
-public class RestComponent implements RestHandlersRegister, IgniteComponent {
+@OpenAPIDefinition(info = @Info(title = "Ignite REST module", version = "3.0.0-alpha", license = @License(name = "Apache 2.0", url = "https://ignite.apache.org"), contact = @Contact(email = "user@ignite.apache.org")))
+@OpenAPIInclude(classes = {ClusterConfigurationController.class, NodeConfigurationController.class, ClusterManagementController.class})
+public class RestComponent implements IgniteComponent {
+ /** Default port. */
+ public static final int DFLT_PORT = 10300;
+
/** Ignite logger. */
private final IgniteLogger log = IgniteLogger.forClass(RestComponent.class);
- /** Node configuration register. */
- private final ConfigurationRegistry nodeCfgRegistry;
-
- /** Netty bootstrap factory. */
- private final NettyBootstrapFactory bootstrapFactory;
-
- private final SimpleRouter router = new SimpleRouter();
-
- /** Netty channel. */
- private volatile Channel channel;
+ /** Factories that produce beans needed for REST controllers. */
+ private final List<RestFactory> restFactories;
+ private final RestConfiguration restConfiguration;
+ /** Server host. */
+ private final String host = "localhost";
+ /** Micronaut application context. */
+ private ApplicationContext context;
+ /** Server port. */
+ private int port;
/**
* Creates a new instance of REST module.
- *
- * @param nodeCfgMgr Node configuration manager.
- * @param bootstrapFactory Netty bootstrap factory.
*/
- public RestComponent(ConfigurationManager nodeCfgMgr, NettyBootstrapFactory bootstrapFactory) {
- nodeCfgRegistry = nodeCfgMgr.configurationRegistry();
-
- this.bootstrapFactory = bootstrapFactory;
- }
-
- /** {@inheritDoc} */
- @Override
- public void registerHandlers(Consumer<Routes> registerAction) {
- registerAction.accept(router);
+ public RestComponent(List<RestFactory> restFactories, RestConfiguration restConfiguration) {
+ this.restFactories = restFactories;
+ this.restConfiguration = restConfiguration;
}
/** {@inheritDoc} */
@Override
public void start() {
- if (channel != null) {
- throw new IgniteException("RestModule is already started.");
- }
-
- channel = startRestEndpoint(router).channel();
- }
-
- /**
- * Start endpoint.
- *
- * @param router Dispatcher of http requests.
- * @return Future which will be notified when this channel is closed.
- * @throws RuntimeException if this module cannot be bound to a port.
- */
- private ChannelFuture startRestEndpoint(Router router) {
- RestView restConfigurationView = nodeCfgRegistry.getConfiguration(RestConfiguration.KEY).value();
+ RestView restConfigurationView = restConfiguration.value();
int desiredPort = restConfigurationView.port();
int portRange = restConfigurationView.portRange();
- int port = 0;
- Channel ch = null;
-
- ServerBootstrap bootstrap = bootstrapFactory.createServerBootstrap()
- .handler(new LoggingHandler(LogLevel.INFO))
- .childHandler(new RestApiInitializer(router));
-
for (int portCandidate = desiredPort; portCandidate <= desiredPort + portRange; portCandidate++) {
- ChannelFuture bindRes = bootstrap.bind(portCandidate).awaitUninterruptibly();
-
- if (bindRes.isSuccess()) {
- ch = bindRes.channel();
+ try {
+ context = buildMicronautContext(portCandidate).start();
port = portCandidate;
+ log.info("REST protocol started successfully");
break;
Review Comment:
you can replace this with `return` and remove the `context == null` check below
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java:
##########
@@ -17,151 +17,147 @@
package org.apache.ignite.internal.rest;
-import io.netty.bootstrap.ServerBootstrap;
-import io.netty.channel.Channel;
-import io.netty.channel.ChannelFuture;
-import io.netty.handler.logging.LogLevel;
-import io.netty.handler.logging.LoggingHandler;
+import io.micronaut.context.ApplicationContext;
+import io.micronaut.http.server.exceptions.ServerStartupException;
+import io.micronaut.openapi.annotation.OpenAPIInclude;
+import io.micronaut.runtime.Micronaut;
+import io.micronaut.runtime.exceptions.ApplicationStartupException;
+import io.swagger.v3.oas.annotations.OpenAPIDefinition;
+import io.swagger.v3.oas.annotations.info.Contact;
+import io.swagger.v3.oas.annotations.info.Info;
+import io.swagger.v3.oas.annotations.info.License;
import java.net.BindException;
-import java.net.InetSocketAddress;
-import java.util.function.Consumer;
+import java.util.List;
+import java.util.Map;
import org.apache.ignite.configuration.schemas.rest.RestConfiguration;
import org.apache.ignite.configuration.schemas.rest.RestView;
-import org.apache.ignite.internal.configuration.ConfigurationManager;
-import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController;
import org.apache.ignite.internal.manager.IgniteComponent;
-import org.apache.ignite.internal.rest.api.RestHandlersRegister;
-import org.apache.ignite.internal.rest.api.Routes;
-import org.apache.ignite.internal.rest.netty.RestApiInitializer;
-import org.apache.ignite.internal.rest.routes.Router;
-import org.apache.ignite.internal.rest.routes.SimpleRouter;
-import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.internal.rest.api.RestFactory;
+import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController;
+import org.apache.ignite.internal.rest.configuration.NodeConfigurationController;
import org.apache.ignite.lang.IgniteInternalException;
import org.apache.ignite.lang.IgniteLogger;
-import org.apache.ignite.network.NettyBootstrapFactory;
/**
- * Rest component is responsible for starting REST endpoints.
+ * Rest module is responsible for starting a REST endpoints for accessing and managing configuration.
*
- * <p>It is started on port 10300 by default, but it is possible to change this in configuration itself. Refer to default config file in
+ * <p>It is started on port 10300 by default but it is possible to change this in configuration itself. Refer to default config file in
* resources for the example.
*/
-public class RestComponent implements RestHandlersRegister, IgniteComponent {
+@OpenAPIDefinition(info = @Info(title = "Ignite REST module", version = "3.0.0-alpha", license = @License(name = "Apache 2.0", url = "https://ignite.apache.org"), contact = @Contact(email = "user@ignite.apache.org")))
+@OpenAPIInclude(classes = {ClusterConfigurationController.class, NodeConfigurationController.class, ClusterManagementController.class})
+public class RestComponent implements IgniteComponent {
+ /** Default port. */
+ public static final int DFLT_PORT = 10300;
+
/** Ignite logger. */
private final IgniteLogger log = IgniteLogger.forClass(RestComponent.class);
- /** Node configuration register. */
- private final ConfigurationRegistry nodeCfgRegistry;
-
- /** Netty bootstrap factory. */
- private final NettyBootstrapFactory bootstrapFactory;
-
- private final SimpleRouter router = new SimpleRouter();
-
- /** Netty channel. */
- private volatile Channel channel;
+ /** Factories that produce beans needed for REST controllers. */
+ private final List<RestFactory> restFactories;
+ private final RestConfiguration restConfiguration;
+ /** Server host. */
+ private final String host = "localhost";
Review Comment:
Why is this not a static constant?
##########
modules/cluster-management/src/integrationTest/java/org/apache/ignite/internal/cluster/management/rest/ItClusterManagementControllerTest.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * 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.cluster.management.rest;
+
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import io.micronaut.context.annotation.Bean;
+import io.micronaut.context.annotation.Factory;
+import io.micronaut.context.annotation.Replaces;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpResponse;
+import io.micronaut.http.HttpStatus;
+import io.micronaut.http.client.HttpClient;
+import io.micronaut.http.client.annotation.Client;
+import io.micronaut.http.client.exceptions.HttpClientResponseException;
+import io.micronaut.runtime.server.EmbeddedServer;
+import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
+import jakarta.inject.Inject;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.cluster.management.ClusterInitializer;
+import org.apache.ignite.internal.cluster.management.MockNode;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.apache.ignite.network.ClusterService;
+import org.apache.ignite.network.NetworkAddress;
+import org.apache.ignite.network.StaticNodeFinder;
+import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * Cluster management REST test.
+ */
+@MicronautTest
+@ExtendWith(WorkDirectoryExtension.class)
+public class ItClusterManagementControllerTest {
+
+ private static final int PORT_BASE = 10000;
+
+ private static final List<MockNode> cluster = new ArrayList<>();
+ static ClusterService clusterService;
+ @WorkDirectory
+ private static Path workDir;
+ @Inject
+ EmbeddedServer server;
Review Comment:
Is it possible to inject into private fields? If yes, please make them private
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/ClusterManagementController.java:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.cluster.management.rest;
+
+import io.micronaut.http.MediaType;
+import io.micronaut.http.annotation.Body;
+import io.micronaut.http.annotation.Consumes;
+import io.micronaut.http.annotation.Controller;
+import io.micronaut.http.annotation.Post;
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import java.util.concurrent.ExecutionException;
+import org.apache.ignite.internal.cluster.management.ClusterInitializer;
+import org.apache.ignite.internal.cluster.management.rest.exception.InvalidNodesInitClusterException;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.IgniteInternalException;
+import org.apache.ignite.lang.IgniteLogger;
+
+/**
+ * Cluster management controller.
+ */
+@Controller("/management/v1/cluster/init")
+@ApiResponse(responseCode = "400", description = "Incorrect body")
+@ApiResponse(responseCode = "500", description = "Internal error")
+@Tag(name = "clusterManagement")
+public class ClusterManagementController {
+ private static final IgniteLogger log = IgniteLogger.forClass(ClusterManagementController.class);
+
+ private final ClusterInitializer clusterInitializer;
+
+ /**
+ * Cluster management controller constructor.
+ *
+ * @param clusterInitializer cluster initializer.
+ */
+ public ClusterManagementController(ClusterInitializer clusterInitializer) {
+ this.clusterInitializer = clusterInitializer;
+ }
+
+ /**
+ * Initializes cluster.
+ */
+ @Post
+ @Operation(operationId = "init")
+ @ApiResponse(responseCode = "200", description = "Cluster initialized")
+ @ApiResponse(responseCode = "409", description = "Cluster already initialized")
+ @Consumes(MediaType.APPLICATION_JSON)
+ public void init(@Body InitCommand initCommand) throws ExecutionException, InterruptedException {
+ if (log.isInfoEnabled()) {
+ log.info("Received init command:\n\tMeta Storage nodes: {}\n\tCMG nodes: {}", initCommand.metaStorageNodes(),
+ initCommand.cmgNodes());
+ }
+
+ try {
+ clusterInitializer.initCluster(initCommand.metaStorageNodes(), initCommand.cmgNodes(), initCommand.clusterName()).get();
Review Comment:
How does Micronaut actually dispatch user requests on top of Netty? Do we still serve them in a single thread? Is it ok to use long running operations in controller methods?
##########
modules/cli/src/integrationTest/resources/hardcoded-ports-config.json:
##########
@@ -8,6 +8,6 @@
},
"rest": {
"port": <REST_PORT>,
- "portRange": 0
+ "portRange": 5
Review Comment:
Why is this change needed?
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalExceptionHandler.java:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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.cluster.management.rest.exception.handler;
+
+import io.micronaut.context.annotation.Requires;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpResponse;
+import io.micronaut.http.HttpResponseFactory;
+import io.micronaut.http.HttpStatus;
+import io.micronaut.http.annotation.Produces;
+import io.micronaut.http.server.exceptions.ExceptionHandler;
+import jakarta.inject.Singleton;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.apache.ignite.lang.IgniteInternalException;
+
+/**
+ * Handles {@link IgniteInternalException} and represents it as a rest response.
+ */
+@Produces
+@Singleton
+@Requires(classes = {IgniteInternalException.class, ExceptionHandler.class})
+public class IgniteInternalExceptionHandler implements ExceptionHandler<IgniteInternalException, HttpResponse<ErrorResult>> {
+
+ @Override
+ public HttpResponse<ErrorResult> handle(HttpRequest request, IgniteInternalException exception) {
+ final ErrorResult errorResult = new ErrorResult("ALREADY_INITIALIZED", exception.getMessage());
Review Comment:
```suggestion
ErrorResult errorResult = new ErrorResult("ALREADY_INITIALIZED", exception.getMessage());
```
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalExceptionHandler.java:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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.cluster.management.rest.exception.handler;
+
+import io.micronaut.context.annotation.Requires;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpResponse;
+import io.micronaut.http.HttpResponseFactory;
+import io.micronaut.http.HttpStatus;
+import io.micronaut.http.annotation.Produces;
+import io.micronaut.http.server.exceptions.ExceptionHandler;
+import jakarta.inject.Singleton;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.apache.ignite.lang.IgniteInternalException;
+
+/**
+ * Handles {@link IgniteInternalException} and represents it as a rest response.
+ */
+@Produces
+@Singleton
+@Requires(classes = {IgniteInternalException.class, ExceptionHandler.class})
+public class IgniteInternalExceptionHandler implements ExceptionHandler<IgniteInternalException, HttpResponse<ErrorResult>> {
+
+ @Override
+ public HttpResponse<ErrorResult> handle(HttpRequest request, IgniteInternalException exception) {
+ final ErrorResult errorResult = new ErrorResult("ALREADY_INITIALIZED", exception.getMessage());
+ return HttpResponseFactory.INSTANCE.status(HttpStatus.CONFLICT).body(errorResult);
Review Comment:
Why `CONFLICT`?
##########
modules/configuration/src/test/java/org/apache/ignite/internal/rest/configuration/ConfigurationControllerBaseTest.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.ignite.internal.rest.configuration;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import io.micronaut.context.ApplicationContext;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpStatus;
+import io.micronaut.http.MediaType;
+import io.micronaut.http.client.HttpClient;
+import io.micronaut.http.client.exceptions.HttpClientResponseException;
+import io.micronaut.runtime.server.EmbeddedServer;
+import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
+import jakarta.inject.Inject;
+import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation;
+import org.apache.ignite.internal.configuration.rest.presentation.TestRootConfiguration;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * The base test for configuration controllers.
+ */
+@MicronautTest
+public abstract class ConfigurationControllerBaseTest {
+ @Inject
+ EmbeddedServer server;
+
+ @Inject
+ ConfigurationPresentation<String> cfgPresentation;
+
+ @Inject
+ ConfigurationRegistry configurationRegistry;
+
+ @Inject
+ ApplicationContext context;
+
+ abstract HttpClient client();
+
+ @BeforeEach
+ void beforeEach() throws Exception {
+ var cfg = configurationRegistry.getConfiguration(TestRootConfiguration.KEY);
+ cfg.change(c -> c.changeFoo("foo").changeSubCfg(subCfg -> subCfg.changeBar("bar"))).get(1, SECONDS);
+ }
+
+ @Test
+ void testGetConfig() {
+ var response = client().toBlocking().exchange("", String.class);
+
+ assertEquals(HttpStatus.OK, response.status());
+ assertEquals(cfgPresentation.represent(), response.body());
+ }
+
+ @Test
+ void testGetConfigByPath() {
+ var response = client().toBlocking().exchange("/root.subCfg", String.class);
+
+ assertEquals(HttpStatus.OK, response.status());
+ assertEquals(cfgPresentation.representByPath("root.subCfg"), response.body());
+ }
+
+ @Test
+ void testUpdateConfig() {
+ String givenChangedConfig = "{root:{foo:foo,subCfg:{bar:changed}}}";
+
+ var response = client().toBlocking().exchange(
+ HttpRequest.PATCH("", givenChangedConfig).contentType(MediaType.TEXT_PLAIN)
+ );
+ assertEquals(response.status(), HttpStatus.OK);
+
+ String changedConfigValue = client().toBlocking().exchange("/root.subCfg.bar", String.class).body();
+ assertEquals("\"changed\"", changedConfigValue);
+ }
+
+ @Test
+ void testUnrecognizedConfigPath() {
+ var thrown = assertThrows(
+ HttpClientResponseException.class,
+ () -> client().toBlocking().exchange("/no-such-root.some-value")
+ );
+
+ assertEquals(HttpStatus.BAD_REQUEST, thrown.getResponse().status());
+
+ var errorResult = getErrorResult(thrown);
+ assertEquals("CONFIG_PATH_UNRECOGNIZED", errorResult.type());
+ assertTrue(errorResult.message().contains("no-such-root"));
Review Comment:
I think it's better to validate whole error messages
##########
modules/configuration/src/main/java/org/apache/ignite/internal/rest/configuration/NodeConfigurationController.java:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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.rest.configuration;
+
+import io.micronaut.http.MediaType;
+import io.micronaut.http.annotation.Body;
+import io.micronaut.http.annotation.Consumes;
+import io.micronaut.http.annotation.Controller;
+import io.micronaut.http.annotation.Get;
+import io.micronaut.http.annotation.Patch;
+import io.micronaut.http.annotation.PathVariable;
+import io.micronaut.http.annotation.Produces;
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.media.Content;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import jakarta.inject.Named;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation;
+
+/**
+ * Node configuration controller.
+ */
+@Controller("/management/v1/configuration/node")
+@ApiResponse(responseCode = "400", description = "Incorrect configuration")
+@ApiResponse(responseCode = "500", description = "Internal error")
+@Tag(name = "nodeConfiguration")
+public class NodeConfigurationController extends AbstractConfigurationController {
+
+ public NodeConfigurationController(@Named("nodeCfgPresentation") ConfigurationPresentation<String> nodeCfgPresentation) {
+ super(nodeCfgPresentation);
+ }
+
+ /**
+ * Returns node configuration in HOCON format. This is represented as a plain text.
+ *
+ * @return the whole node configuration in HOCON format.
+ */
+ @Operation(operationId = "getNodeConfiguration")
+ @ApiResponse(responseCode = "200",
+ content = @Content(mediaType = MediaType.TEXT_PLAIN,
+ schema = @Schema(type = "string")),
+ description = "Whole node configuration")
+ @Produces(MediaType.TEXT_PLAIN)
+ @Get
+ public String getConfiguration() {
Review Comment:
Same here: missing `@Override`
##########
modules/configuration/src/main/java/org/apache/ignite/internal/rest/configuration/ClusterConfigurationController.java:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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.rest.configuration;
+
+import io.micronaut.http.MediaType;
+import io.micronaut.http.annotation.Body;
+import io.micronaut.http.annotation.Consumes;
+import io.micronaut.http.annotation.Controller;
+import io.micronaut.http.annotation.Get;
+import io.micronaut.http.annotation.Patch;
+import io.micronaut.http.annotation.PathVariable;
+import io.micronaut.http.annotation.Produces;
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.media.Content;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import jakarta.inject.Named;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation;
+
+/**
+ * Cluster configuration controller.
+ */
+@Controller("/management/v1/configuration/cluster/")
+@ApiResponse(responseCode = "400", description = "Incorrect configuration")
+@ApiResponse(responseCode = "500", description = "Internal error")
+@Tag(name = "clusterConfiguration")
+public class ClusterConfigurationController extends AbstractConfigurationController {
+
+ public ClusterConfigurationController(@Named("clusterCfgPresentation") ConfigurationPresentation<String> clusterCfgPresentation) {
+ super(clusterCfgPresentation);
+ }
+
+ /**
+ * Returns cluster configuration in HOCON format. This is represented as a plain text.
+ *
+ * @return the whole cluster configuration in HOCON format.
+ */
+ @Operation(operationId = "getClusterConfiguration")
+ @ApiResponse(
+ responseCode = "200",
+ content = @Content(mediaType = MediaType.TEXT_PLAIN, schema = @Schema(type = "string")),
+ description = "Get cluster configuration")
+ @Produces(MediaType.TEXT_PLAIN)
+ @Get
+ public String getConfiguration() {
Review Comment:
missing `@Override` here and on other methods
##########
modules/configuration/src/test/java/org/apache/ignite/internal/rest/configuration/ConfigurationControllerBaseTest.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.ignite.internal.rest.configuration;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import io.micronaut.context.ApplicationContext;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpStatus;
+import io.micronaut.http.MediaType;
+import io.micronaut.http.client.HttpClient;
+import io.micronaut.http.client.exceptions.HttpClientResponseException;
+import io.micronaut.runtime.server.EmbeddedServer;
+import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
+import jakarta.inject.Inject;
+import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation;
+import org.apache.ignite.internal.configuration.rest.presentation.TestRootConfiguration;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * The base test for configuration controllers.
+ */
+@MicronautTest
+public abstract class ConfigurationControllerBaseTest {
+ @Inject
+ EmbeddedServer server;
Review Comment:
Same question: can these fields be `private`?
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java:
##########
@@ -17,151 +17,147 @@
package org.apache.ignite.internal.rest;
-import io.netty.bootstrap.ServerBootstrap;
-import io.netty.channel.Channel;
-import io.netty.channel.ChannelFuture;
-import io.netty.handler.logging.LogLevel;
-import io.netty.handler.logging.LoggingHandler;
+import io.micronaut.context.ApplicationContext;
+import io.micronaut.http.server.exceptions.ServerStartupException;
+import io.micronaut.openapi.annotation.OpenAPIInclude;
+import io.micronaut.runtime.Micronaut;
+import io.micronaut.runtime.exceptions.ApplicationStartupException;
+import io.swagger.v3.oas.annotations.OpenAPIDefinition;
+import io.swagger.v3.oas.annotations.info.Contact;
+import io.swagger.v3.oas.annotations.info.Info;
+import io.swagger.v3.oas.annotations.info.License;
import java.net.BindException;
-import java.net.InetSocketAddress;
-import java.util.function.Consumer;
+import java.util.List;
+import java.util.Map;
import org.apache.ignite.configuration.schemas.rest.RestConfiguration;
import org.apache.ignite.configuration.schemas.rest.RestView;
-import org.apache.ignite.internal.configuration.ConfigurationManager;
-import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController;
import org.apache.ignite.internal.manager.IgniteComponent;
-import org.apache.ignite.internal.rest.api.RestHandlersRegister;
-import org.apache.ignite.internal.rest.api.Routes;
-import org.apache.ignite.internal.rest.netty.RestApiInitializer;
-import org.apache.ignite.internal.rest.routes.Router;
-import org.apache.ignite.internal.rest.routes.SimpleRouter;
-import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.internal.rest.api.RestFactory;
+import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController;
+import org.apache.ignite.internal.rest.configuration.NodeConfigurationController;
import org.apache.ignite.lang.IgniteInternalException;
import org.apache.ignite.lang.IgniteLogger;
-import org.apache.ignite.network.NettyBootstrapFactory;
/**
- * Rest component is responsible for starting REST endpoints.
+ * Rest module is responsible for starting a REST endpoints for accessing and managing configuration.
*
- * <p>It is started on port 10300 by default, but it is possible to change this in configuration itself. Refer to default config file in
+ * <p>It is started on port 10300 by default but it is possible to change this in configuration itself. Refer to default config file in
* resources for the example.
*/
-public class RestComponent implements RestHandlersRegister, IgniteComponent {
+@OpenAPIDefinition(info = @Info(title = "Ignite REST module", version = "3.0.0-alpha", license = @License(name = "Apache 2.0", url = "https://ignite.apache.org"), contact = @Contact(email = "user@ignite.apache.org")))
+@OpenAPIInclude(classes = {ClusterConfigurationController.class, NodeConfigurationController.class, ClusterManagementController.class})
+public class RestComponent implements IgniteComponent {
+ /** Default port. */
+ public static final int DFLT_PORT = 10300;
+
/** Ignite logger. */
private final IgniteLogger log = IgniteLogger.forClass(RestComponent.class);
- /** Node configuration register. */
- private final ConfigurationRegistry nodeCfgRegistry;
-
- /** Netty bootstrap factory. */
- private final NettyBootstrapFactory bootstrapFactory;
-
- private final SimpleRouter router = new SimpleRouter();
-
- /** Netty channel. */
- private volatile Channel channel;
+ /** Factories that produce beans needed for REST controllers. */
+ private final List<RestFactory> restFactories;
+ private final RestConfiguration restConfiguration;
+ /** Server host. */
+ private final String host = "localhost";
+ /** Micronaut application context. */
+ private ApplicationContext context;
Review Comment:
I would suggest to mark `context` and `port` as `volatile`
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java:
##########
@@ -17,151 +17,147 @@
package org.apache.ignite.internal.rest;
-import io.netty.bootstrap.ServerBootstrap;
-import io.netty.channel.Channel;
-import io.netty.channel.ChannelFuture;
-import io.netty.handler.logging.LogLevel;
-import io.netty.handler.logging.LoggingHandler;
+import io.micronaut.context.ApplicationContext;
+import io.micronaut.http.server.exceptions.ServerStartupException;
+import io.micronaut.openapi.annotation.OpenAPIInclude;
+import io.micronaut.runtime.Micronaut;
+import io.micronaut.runtime.exceptions.ApplicationStartupException;
+import io.swagger.v3.oas.annotations.OpenAPIDefinition;
+import io.swagger.v3.oas.annotations.info.Contact;
+import io.swagger.v3.oas.annotations.info.Info;
+import io.swagger.v3.oas.annotations.info.License;
import java.net.BindException;
-import java.net.InetSocketAddress;
-import java.util.function.Consumer;
+import java.util.List;
+import java.util.Map;
import org.apache.ignite.configuration.schemas.rest.RestConfiguration;
import org.apache.ignite.configuration.schemas.rest.RestView;
-import org.apache.ignite.internal.configuration.ConfigurationManager;
-import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController;
import org.apache.ignite.internal.manager.IgniteComponent;
-import org.apache.ignite.internal.rest.api.RestHandlersRegister;
-import org.apache.ignite.internal.rest.api.Routes;
-import org.apache.ignite.internal.rest.netty.RestApiInitializer;
-import org.apache.ignite.internal.rest.routes.Router;
-import org.apache.ignite.internal.rest.routes.SimpleRouter;
-import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.internal.rest.api.RestFactory;
+import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController;
+import org.apache.ignite.internal.rest.configuration.NodeConfigurationController;
import org.apache.ignite.lang.IgniteInternalException;
import org.apache.ignite.lang.IgniteLogger;
-import org.apache.ignite.network.NettyBootstrapFactory;
/**
- * Rest component is responsible for starting REST endpoints.
+ * Rest module is responsible for starting a REST endpoints for accessing and managing configuration.
Review Comment:
```suggestion
* Rest module is responsible for starting REST endpoints for accessing and managing configuration.
```
##########
modules/rest/pom.xml:
##########
@@ -151,7 +128,21 @@
<artifactId>ignite-configuration-annotation-processor</artifactId>
<version>${project.version}</version>
</path>
+ <path>
+ <groupId>io.micronaut</groupId>
+ <artifactId>micronaut-inject-java</artifactId>
+ <version>${micronaut.version}</version>
+ </path>
+ <path>
+ <groupId>io.micronaut.openapi</groupId>
+ <artifactId>micronaut-openapi</artifactId>
+ <version>${maven.micronaut.openapi.plugin.version}</version>
+ </path>
</annotationProcessorPaths>
+ <fork>true</fork>
Review Comment:
what is this parameter for?
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java:
##########
@@ -17,151 +17,147 @@
package org.apache.ignite.internal.rest;
-import io.netty.bootstrap.ServerBootstrap;
-import io.netty.channel.Channel;
-import io.netty.channel.ChannelFuture;
-import io.netty.handler.logging.LogLevel;
-import io.netty.handler.logging.LoggingHandler;
+import io.micronaut.context.ApplicationContext;
+import io.micronaut.http.server.exceptions.ServerStartupException;
+import io.micronaut.openapi.annotation.OpenAPIInclude;
+import io.micronaut.runtime.Micronaut;
+import io.micronaut.runtime.exceptions.ApplicationStartupException;
+import io.swagger.v3.oas.annotations.OpenAPIDefinition;
+import io.swagger.v3.oas.annotations.info.Contact;
+import io.swagger.v3.oas.annotations.info.Info;
+import io.swagger.v3.oas.annotations.info.License;
import java.net.BindException;
-import java.net.InetSocketAddress;
-import java.util.function.Consumer;
+import java.util.List;
+import java.util.Map;
import org.apache.ignite.configuration.schemas.rest.RestConfiguration;
import org.apache.ignite.configuration.schemas.rest.RestView;
-import org.apache.ignite.internal.configuration.ConfigurationManager;
-import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController;
import org.apache.ignite.internal.manager.IgniteComponent;
-import org.apache.ignite.internal.rest.api.RestHandlersRegister;
-import org.apache.ignite.internal.rest.api.Routes;
-import org.apache.ignite.internal.rest.netty.RestApiInitializer;
-import org.apache.ignite.internal.rest.routes.Router;
-import org.apache.ignite.internal.rest.routes.SimpleRouter;
-import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.internal.rest.api.RestFactory;
+import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController;
+import org.apache.ignite.internal.rest.configuration.NodeConfigurationController;
import org.apache.ignite.lang.IgniteInternalException;
import org.apache.ignite.lang.IgniteLogger;
-import org.apache.ignite.network.NettyBootstrapFactory;
/**
- * Rest component is responsible for starting REST endpoints.
+ * Rest module is responsible for starting a REST endpoints for accessing and managing configuration.
*
- * <p>It is started on port 10300 by default, but it is possible to change this in configuration itself. Refer to default config file in
+ * <p>It is started on port 10300 by default but it is possible to change this in configuration itself. Refer to default config file in
* resources for the example.
*/
-public class RestComponent implements RestHandlersRegister, IgniteComponent {
+@OpenAPIDefinition(info = @Info(title = "Ignite REST module", version = "3.0.0-alpha", license = @License(name = "Apache 2.0", url = "https://ignite.apache.org"), contact = @Contact(email = "user@ignite.apache.org")))
+@OpenAPIInclude(classes = {ClusterConfigurationController.class, NodeConfigurationController.class, ClusterManagementController.class})
+public class RestComponent implements IgniteComponent {
+ /** Default port. */
+ public static final int DFLT_PORT = 10300;
+
/** Ignite logger. */
private final IgniteLogger log = IgniteLogger.forClass(RestComponent.class);
- /** Node configuration register. */
- private final ConfigurationRegistry nodeCfgRegistry;
-
- /** Netty bootstrap factory. */
- private final NettyBootstrapFactory bootstrapFactory;
-
- private final SimpleRouter router = new SimpleRouter();
-
- /** Netty channel. */
- private volatile Channel channel;
+ /** Factories that produce beans needed for REST controllers. */
+ private final List<RestFactory> restFactories;
+ private final RestConfiguration restConfiguration;
+ /** Server host. */
+ private final String host = "localhost";
+ /** Micronaut application context. */
+ private ApplicationContext context;
Review Comment:
Actually, `port` can be left as-is if you carefully guard it with `context` accesses
##########
modules/cluster-management/src/integrationTest/java/org/apache/ignite/internal/cluster/management/rest/ItClusterManagementControllerTest.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * 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.cluster.management.rest;
+
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import io.micronaut.context.annotation.Bean;
+import io.micronaut.context.annotation.Factory;
+import io.micronaut.context.annotation.Replaces;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpResponse;
+import io.micronaut.http.HttpStatus;
+import io.micronaut.http.client.HttpClient;
+import io.micronaut.http.client.annotation.Client;
+import io.micronaut.http.client.exceptions.HttpClientResponseException;
+import io.micronaut.runtime.server.EmbeddedServer;
+import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
+import jakarta.inject.Inject;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.cluster.management.ClusterInitializer;
+import org.apache.ignite.internal.cluster.management.MockNode;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.apache.ignite.network.ClusterService;
+import org.apache.ignite.network.NetworkAddress;
+import org.apache.ignite.network.StaticNodeFinder;
+import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * Cluster management REST test.
+ */
+@MicronautTest
+@ExtendWith(WorkDirectoryExtension.class)
+public class ItClusterManagementControllerTest {
+
+ private static final int PORT_BASE = 10000;
+
+ private static final List<MockNode> cluster = new ArrayList<>();
+ static ClusterService clusterService;
+ @WorkDirectory
+ private static Path workDir;
+ @Inject
+ EmbeddedServer server;
+ @Inject
+ @Client("/management/v1/cluster/init/")
+ HttpClient client;
+ @Inject
+ ClusterInitializer clusterInitializer;
Review Comment:
This field is not used
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java:
##########
@@ -17,151 +17,147 @@
package org.apache.ignite.internal.rest;
-import io.netty.bootstrap.ServerBootstrap;
-import io.netty.channel.Channel;
-import io.netty.channel.ChannelFuture;
-import io.netty.handler.logging.LogLevel;
-import io.netty.handler.logging.LoggingHandler;
+import io.micronaut.context.ApplicationContext;
+import io.micronaut.http.server.exceptions.ServerStartupException;
+import io.micronaut.openapi.annotation.OpenAPIInclude;
+import io.micronaut.runtime.Micronaut;
+import io.micronaut.runtime.exceptions.ApplicationStartupException;
+import io.swagger.v3.oas.annotations.OpenAPIDefinition;
+import io.swagger.v3.oas.annotations.info.Contact;
+import io.swagger.v3.oas.annotations.info.Info;
+import io.swagger.v3.oas.annotations.info.License;
import java.net.BindException;
-import java.net.InetSocketAddress;
-import java.util.function.Consumer;
+import java.util.List;
+import java.util.Map;
import org.apache.ignite.configuration.schemas.rest.RestConfiguration;
import org.apache.ignite.configuration.schemas.rest.RestView;
-import org.apache.ignite.internal.configuration.ConfigurationManager;
-import org.apache.ignite.internal.configuration.ConfigurationRegistry;
+import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController;
import org.apache.ignite.internal.manager.IgniteComponent;
-import org.apache.ignite.internal.rest.api.RestHandlersRegister;
-import org.apache.ignite.internal.rest.api.Routes;
-import org.apache.ignite.internal.rest.netty.RestApiInitializer;
-import org.apache.ignite.internal.rest.routes.Router;
-import org.apache.ignite.internal.rest.routes.SimpleRouter;
-import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.internal.rest.api.RestFactory;
+import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController;
+import org.apache.ignite.internal.rest.configuration.NodeConfigurationController;
import org.apache.ignite.lang.IgniteInternalException;
import org.apache.ignite.lang.IgniteLogger;
-import org.apache.ignite.network.NettyBootstrapFactory;
/**
- * Rest component is responsible for starting REST endpoints.
+ * Rest module is responsible for starting a REST endpoints for accessing and managing configuration.
*
- * <p>It is started on port 10300 by default, but it is possible to change this in configuration itself. Refer to default config file in
+ * <p>It is started on port 10300 by default but it is possible to change this in configuration itself. Refer to default config file in
* resources for the example.
*/
-public class RestComponent implements RestHandlersRegister, IgniteComponent {
+@OpenAPIDefinition(info = @Info(title = "Ignite REST module", version = "3.0.0-alpha", license = @License(name = "Apache 2.0", url = "https://ignite.apache.org"), contact = @Contact(email = "user@ignite.apache.org")))
+@OpenAPIInclude(classes = {ClusterConfigurationController.class, NodeConfigurationController.class, ClusterManagementController.class})
+public class RestComponent implements IgniteComponent {
+ /** Default port. */
+ public static final int DFLT_PORT = 10300;
+
/** Ignite logger. */
private final IgniteLogger log = IgniteLogger.forClass(RestComponent.class);
- /** Node configuration register. */
- private final ConfigurationRegistry nodeCfgRegistry;
-
- /** Netty bootstrap factory. */
- private final NettyBootstrapFactory bootstrapFactory;
-
- private final SimpleRouter router = new SimpleRouter();
-
- /** Netty channel. */
- private volatile Channel channel;
+ /** Factories that produce beans needed for REST controllers. */
+ private final List<RestFactory> restFactories;
+ private final RestConfiguration restConfiguration;
+ /** Server host. */
+ private final String host = "localhost";
+ /** Micronaut application context. */
+ private ApplicationContext context;
+ /** Server port. */
+ private int port;
/**
* Creates a new instance of REST module.
- *
- * @param nodeCfgMgr Node configuration manager.
- * @param bootstrapFactory Netty bootstrap factory.
*/
- public RestComponent(ConfigurationManager nodeCfgMgr, NettyBootstrapFactory bootstrapFactory) {
- nodeCfgRegistry = nodeCfgMgr.configurationRegistry();
-
- this.bootstrapFactory = bootstrapFactory;
- }
-
- /** {@inheritDoc} */
- @Override
- public void registerHandlers(Consumer<Routes> registerAction) {
- registerAction.accept(router);
+ public RestComponent(List<RestFactory> restFactories, RestConfiguration restConfiguration) {
+ this.restFactories = restFactories;
+ this.restConfiguration = restConfiguration;
}
/** {@inheritDoc} */
@Override
public void start() {
- if (channel != null) {
- throw new IgniteException("RestModule is already started.");
- }
-
- channel = startRestEndpoint(router).channel();
- }
-
- /**
- * Start endpoint.
- *
- * @param router Dispatcher of http requests.
- * @return Future which will be notified when this channel is closed.
- * @throws RuntimeException if this module cannot be bound to a port.
- */
- private ChannelFuture startRestEndpoint(Router router) {
- RestView restConfigurationView = nodeCfgRegistry.getConfiguration(RestConfiguration.KEY).value();
+ RestView restConfigurationView = restConfiguration.value();
int desiredPort = restConfigurationView.port();
int portRange = restConfigurationView.portRange();
- int port = 0;
- Channel ch = null;
-
- ServerBootstrap bootstrap = bootstrapFactory.createServerBootstrap()
- .handler(new LoggingHandler(LogLevel.INFO))
- .childHandler(new RestApiInitializer(router));
-
for (int portCandidate = desiredPort; portCandidate <= desiredPort + portRange; portCandidate++) {
- ChannelFuture bindRes = bootstrap.bind(portCandidate).awaitUninterruptibly();
-
- if (bindRes.isSuccess()) {
- ch = bindRes.channel();
+ try {
+ context = buildMicronautContext(portCandidate).start();
port = portCandidate;
+ log.info("REST protocol started successfully");
break;
- } else if (!(bindRes.cause() instanceof BindException)) {
- throw new RuntimeException(bindRes.cause());
+ } catch (ApplicationStartupException e) {
+ log.error("Got exception " + e.getCause() + " during node start on port " + portCandidate + " , trying again");
Review Comment:
That's a bit strange. Are we sure that if we get this exception then the only reason for it is that the target port is busy?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] vveider merged pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
vveider merged PR #816:
URL: https://github.com/apache/ignite-3/pull/816
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r887822712
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalExceptionHandler.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * 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.cluster.management.rest.exception.handler;
+
+import io.micronaut.context.annotation.Requires;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpResponse;
+import io.micronaut.http.annotation.Produces;
+import io.micronaut.http.server.exceptions.ExceptionHandler;
+import jakarta.inject.Singleton;
+import org.apache.ignite.internal.rest.api.ErrorResult;
+import org.apache.ignite.lang.IgniteInternalException;
+
+/**
+ * Handles {@link IgniteInternalException} and represents it as a rest response.
+ */
+@Produces
+@Singleton
+@Requires(classes = {IgniteInternalException.class, ExceptionHandler.class})
+public class IgniteInternalExceptionHandler implements ExceptionHandler<IgniteInternalException, HttpResponse<ErrorResult>> {
+
+ @Override
+ public HttpResponse<ErrorResult> handle(HttpRequest request, IgniteInternalException exception) {
+ ErrorResult errorResult = new ErrorResult("INITIALIZATION_EXCEPTION", exception.getMessage());
+ return HttpResponse.badRequest().body(errorResult);
Review Comment:
In future we have to introduce specific exceptions for situations like: already initialized, initializing, etc.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r885083833
##########
modules/configuration/src/main/java/org/apache/ignite/internal/rest/configuration/ClusterConfigurationController.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.rest.configuration;
+
+import io.micronaut.http.MediaType;
+import io.micronaut.http.annotation.Body;
+import io.micronaut.http.annotation.Consumes;
+import io.micronaut.http.annotation.Controller;
+import io.micronaut.http.annotation.Get;
+import io.micronaut.http.annotation.Patch;
+import io.micronaut.http.annotation.PathVariable;
+import io.micronaut.http.annotation.Produces;
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.media.Content;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import jakarta.inject.Named;
+import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation;
+
+/**
+ * Cluster configuration controller.
+ */
+@Controller("/management/v1/configuration/cluster/")
+@ApiResponse(responseCode = "400", description = "Incorrect configuration")
+@ApiResponse(responseCode = "500", description = "Internal error")
+@Tag(name = "clusterConfiguration")
+public class ClusterConfigurationController extends AbstractConfigurationController {
+
+ public ClusterConfigurationController(@Named("clusterCfgPresentation") ConfigurationPresentation<String> clusterCfgPresentation) {
+ super(clusterCfgPresentation);
+ }
+
+ /**
+ * Returns cluster configuration in HOCON format.
+ *
+ * @return the whole cluster configuration in HOCON format.
+ */
+ @Operation(operationId = "getClusterConfiguration")
+ @ApiResponse(
+ responseCode = "200",
+ content = @Content(mediaType = MediaType.TEXT_PLAIN, schema = @Schema(type = "string")),
+ description = "Get cluster configuration")
+ @Produces(MediaType.TEXT_PLAIN)
+ @Get
+ public String getConfiguration() {
+ return super.getConfiguration();
+ }
+
+ /**
+ * Returns configuration in HOCON format represented by path.
+ *
+ * @param path to represent a cluster configuration.
+ * @return system configuration in HOCON format represented by given path.
+ */
+ @Operation(operationId = "getClusterConfigurationByPath")
+ @ApiResponse(responseCode = "200",
+ content = @Content(mediaType = MediaType.TEXT_PLAIN,
+ schema = @Schema(type = "string")),
+ description = "Configuration represented by path")
+ @Produces(MediaType.TEXT_PLAIN)
+ @Get("/{path}")
+ public String getConfigurationByPath(@PathVariable String path) {
+ return super.getConfigurationByPath(path);
+ }
+
+ /**
+ * Updates cluster configuration in HOCON format.
+ *
+ * @param updatedConfiguration the cluster configuration to update.
+ */
+ @Operation(operationId = "updateClusterConfiguration")
+ @ApiResponse(responseCode = "200", description = "Configuration updated")
+ @Consumes(MediaType.TEXT_PLAIN)
+ @Patch
+ public void updateConfiguration(@Body String updatedConfiguration) throws Throwable {
Review Comment:
Good point, will try.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r885414017
##########
modules/cli/src/main/java/org/apache/ignite/cli/builtins/config/ConfigurationClient.java:
##########
@@ -81,7 +81,7 @@ public String get(
) {
var req = HttpRequest
.newBuilder()
- .header("Content-Type", "application/json");
+ .header("Content-Type", "text/plain");
Review Comment:
Makes sense, can you please leave a comment?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r885453523
##########
modules/rest-client/pom.xml:
##########
@@ -0,0 +1,124 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ 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.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <parent>
+ <groupId>org.apache.ignite</groupId>
+ <artifactId>ignite-parent</artifactId>
+ <version>1</version>
+ <relativePath>../../parent/pom.xml</relativePath>
+ </parent>
+
+ <artifactId>ignite-rest-client</artifactId>
+ <version>3.0.0-SNAPSHOT</version>
+
+ <dependencies>
+
+ <!--client specific dependencies, versions cannot be moved to the parent pom-->
Review Comment:
Actually, this is a valuable note.
This module (just module, not code) and pom are not generated. The plugin section in this pom configures the source code generation. This is done for integration tests for the generated client in the runner module.
From one side, the client should be generated where it is really needed. In our case, it is inside the cli module. But the Open API spec is generated by the rest module and I think that there should be an integration test for the client that is generated from this Open API spec. And these checks have to be done by the server.
I can add a comment to the pom or drop this module and create a ticket for integration tests that will generate the client somewhere inside and not include it as a module.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r885474087
##########
modules/cluster-management/pom.xml:
##########
@@ -49,17 +49,26 @@
<dependency>
<groupId>org.apache.ignite</groupId>
- <artifactId>ignite-rest</artifactId>
+ <artifactId>ignite-vault</artifactId>
</dependency>
<dependency>
<groupId>org.apache.ignite</groupId>
- <artifactId>ignite-vault</artifactId>
+ <artifactId>ignite-rest-api</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>io.micronaut</groupId>
+ <artifactId>micronaut-http</artifactId>
+ <version>${micronaut.version}</version>
Review Comment:
As for micronaut-bom, it might not pass vulnerabilities checks https://mvnrepository.com/artifact/io.micronaut/micronaut-bom/3.5.0.
One more thing is that it declares a huge set of dependencies that are not only micronaut-related. That could be a problem in dependency management.
https://repo1.maven.org/maven2/io/micronaut/micronaut-bom/3.5.0/micronaut-bom-3.5.0.pom
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r885606226
##########
modules/cluster-management/pom.xml:
##########
@@ -49,17 +49,26 @@
<dependency>
<groupId>org.apache.ignite</groupId>
- <artifactId>ignite-rest</artifactId>
+ <artifactId>ignite-vault</artifactId>
</dependency>
<dependency>
<groupId>org.apache.ignite</groupId>
- <artifactId>ignite-vault</artifactId>
+ <artifactId>ignite-rest-api</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>io.micronaut</groupId>
+ <artifactId>micronaut-http</artifactId>
+ <version>${micronaut.version}</version>
Review Comment:
AFAIK, if you declare a dependency directly in the dependency management section, it will override any artifacts declared in imported boms
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r886869530
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/ClusterManagementController.java:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.cluster.management.rest;
+
+import io.micronaut.http.MediaType;
+import io.micronaut.http.annotation.Body;
+import io.micronaut.http.annotation.Consumes;
+import io.micronaut.http.annotation.Controller;
+import io.micronaut.http.annotation.Post;
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import java.util.concurrent.ExecutionException;
+import org.apache.ignite.internal.cluster.management.ClusterInitializer;
+import org.apache.ignite.internal.cluster.management.rest.exception.InvalidNodesInitClusterException;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.IgniteInternalException;
+import org.apache.ignite.lang.IgniteLogger;
+
+/**
+ * Cluster management controller.
+ */
+@Controller("/management/v1/cluster/init")
+@ApiResponse(responseCode = "400", description = "Incorrect body")
+@ApiResponse(responseCode = "500", description = "Internal error")
+@Tag(name = "clusterManagement")
+public class ClusterManagementController {
+ private static final IgniteLogger log = IgniteLogger.forClass(ClusterManagementController.class);
+
+ private final ClusterInitializer clusterInitializer;
+
+ /**
+ * Cluster management controller constructor.
+ *
+ * @param clusterInitializer cluster initializer.
+ */
+ public ClusterManagementController(ClusterInitializer clusterInitializer) {
+ this.clusterInitializer = clusterInitializer;
+ }
+
+ /**
+ * Initializes cluster.
+ */
+ @Post
+ @Operation(operationId = "init")
+ @ApiResponse(responseCode = "200", description = "Cluster initialized")
+ @ApiResponse(responseCode = "409", description = "Cluster already initialized")
+ @Consumes(MediaType.APPLICATION_JSON)
+ public void init(@Body InitCommand initCommand) throws ExecutionException, InterruptedException {
+ if (log.isInfoEnabled()) {
+ log.info("Received init command:\n\tMeta Storage nodes: {}\n\tCMG nodes: {}", initCommand.metaStorageNodes(),
+ initCommand.cmgNodes());
+ }
+
+ try {
+ clusterInitializer.initCluster(initCommand.metaStorageNodes(), initCommand.cmgNodes(), initCommand.clusterName()).get();
Review Comment:
It launches its own thread pool. The number of threads is configurable but it definitely more than 1 by default. So, long-running jobs such as `init` are acceptable.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r887334337
##########
modules/cli/src/integrationTest/java/org/apache/ignite/cli/ItConfigCommandTest.java:
##########
@@ -113,9 +109,7 @@ public void setAndGetWithManualHost() {
assertEquals(0, exitCode);
- DocumentContext document = JsonPath.parse(removeTrailingQuotes(unescapeQuotes(out.toString(UTF_8))));
-
- assertEquals(1, document.read("$.network.shutdownQuietPeriod", Integer.class));
+ assertTrue(out.toString(UTF_8).contains("\"shutdownQuietPeriod\" : 1"));
Review Comment:
> would it be possible to restore the parsing to make sure it's a valid HOCON?
Good point. I'd like to create a ticket for this because it seems like a lot of tests should be adjusted. Also, I think it is better to check in the server tests instead of in CLI tests.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #816: IGNITE-16943: Implement Micronaut REST
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #816:
URL: https://github.com/apache/ignite-3/pull/816#discussion_r886922311
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/ClusterManagementController.java:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.cluster.management.rest;
+
+import io.micronaut.http.MediaType;
+import io.micronaut.http.annotation.Body;
+import io.micronaut.http.annotation.Consumes;
+import io.micronaut.http.annotation.Controller;
+import io.micronaut.http.annotation.Post;
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.responses.ApiResponse;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import java.util.concurrent.ExecutionException;
+import org.apache.ignite.internal.cluster.management.ClusterInitializer;
+import org.apache.ignite.internal.cluster.management.rest.exception.InvalidNodesInitClusterException;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.IgniteInternalException;
+import org.apache.ignite.lang.IgniteLogger;
+
+/**
+ * Cluster management controller.
+ */
+@Controller("/management/v1/cluster/init")
+@ApiResponse(responseCode = "400", description = "Incorrect body")
+@ApiResponse(responseCode = "500", description = "Internal error")
+@Tag(name = "clusterManagement")
+public class ClusterManagementController {
+ private static final IgniteLogger log = IgniteLogger.forClass(ClusterManagementController.class);
+
+ private final ClusterInitializer clusterInitializer;
+
+ /**
+ * Cluster management controller constructor.
+ *
+ * @param clusterInitializer cluster initializer.
+ */
+ public ClusterManagementController(ClusterInitializer clusterInitializer) {
+ this.clusterInitializer = clusterInitializer;
+ }
+
+ /**
+ * Initializes cluster.
+ */
+ @Post
+ @Operation(operationId = "init")
+ @ApiResponse(responseCode = "200", description = "Cluster initialized")
+ @ApiResponse(responseCode = "409", description = "Cluster already initialized")
+ @Consumes(MediaType.APPLICATION_JSON)
+ public void init(@Body InitCommand initCommand) throws ExecutionException, InterruptedException {
+ if (log.isInfoEnabled()) {
+ log.info("Received init command:\n\tMeta Storage nodes: {}\n\tCMG nodes: {}", initCommand.metaStorageNodes(),
+ initCommand.cmgNodes());
+ }
+
+ try {
+ clusterInitializer.initCluster(initCommand.metaStorageNodes(), initCommand.cmgNodes(), initCommand.clusterName()).get();
Review Comment:
ok
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org