frankgh commented on code in PR #225:
URL: https://github.com/apache/cassandra-sidecar/pull/225#discussion_r2140886201


##########
client/src/testFixtures/java/org/apache/cassandra/sidecar/client/SidecarClientTest.java:
##########


Review Comment:
   can we add some unit tests for the client here?



##########
client/src/main/java/org/apache/cassandra/sidecar/client/RequestContext.java:
##########
@@ -563,6 +567,30 @@ public Builder nodeDecommissionRequest()
             return request(NODE_DECOMMISSION_REQUEST);
         }
 
+        /**
+         * Sets the {@code request}  to be a {@link GossipUpdateRequest} for 
the
+         * given {@link NodeCommandRequestPayload.State state}, and returns a 
reference to this Builder enabling method chaining.
+         *
+         * @param state  the desired state for gossip
+         * @return a reference to this Builder
+         */
+        public Builder nodeGossipUpdateRequest(@NotNull 
NodeCommandRequestPayload.State state)
+        {
+            return request(new GossipUpdateRequest(state));
+        }
+
+        /**
+         * Sets the {@code request}  to be a {@link NativeUpdateRequest} for 
the

Review Comment:
   NIT
   ```suggestion
            * Sets the {@code request} to be a {@link NativeUpdateRequest} for 
the
   ```



##########
client/src/main/java/org/apache/cassandra/sidecar/client/RequestContext.java:
##########
@@ -563,6 +567,30 @@ public Builder nodeDecommissionRequest()
             return request(NODE_DECOMMISSION_REQUEST);
         }
 
+        /**
+         * Sets the {@code request}  to be a {@link GossipUpdateRequest} for 
the

Review Comment:
   NIT
   ```suggestion
            * Sets the {@code request} to be a {@link GossipUpdateRequest} for 
the
   ```



##########
client-common/src/main/java/org/apache/cassandra/sidecar/common/request/GossipUpdateRequest.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.cassandra.sidecar.common.request;
+
+import io.netty.handler.codec.http.HttpMethod;
+import org.apache.cassandra.sidecar.common.ApiEndpointsV1;
+import 
org.apache.cassandra.sidecar.common.request.data.NodeCommandRequestPayload;
+
+/**
+ * Gossip update request
+ */
+public class GossipUpdateRequest extends Request
+{
+    private final NodeCommandRequestPayload requestPayload;
+
+    /**
+     * Constructs a gossip update request with the provided parameters
+     *
+     * @param requestPayload { "state": "start" } or { "state": "stop"  }
+     */
+    public GossipUpdateRequest(NodeCommandRequestPayload requestPayload)

Review Comment:
   do we need this constructor? it seems to be unused



##########
server-common/src/main/java/org/apache/cassandra/sidecar/common/server/StorageOperations.java:
##########
@@ -129,4 +129,12 @@ default void outOfRangeDataCleanup(@NotNull String 
keyspace, @NotNull String tab
      * @return the name of the cluster
      */
     String clusterName();
+
+    void stopNativeTransport();

Review Comment:
   can we add javadocs here?



##########
client-common/src/main/java/org/apache/cassandra/sidecar/common/request/data/NodeCommandRequestPayload.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.cassandra.sidecar.common.request.data;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.cassandra.sidecar.common.utils.Preconditions;
+
+/**
+ * Request payload for start/stop operations (gossip, native transport, etc.).
+ *
+ * <p>Valid JSON:</p>
+ * <pre>
+ *   { "state": "start" }
+ *   { "state": "stop"  }
+ * </pre>
+ */
+@JsonInclude(JsonInclude.Include.NON_NULL)
+public class NodeCommandRequestPayload
+{
+    /**
+     * Node Command State
+     */
+    public enum State
+    {
+        START, STOP;
+
+        @JsonCreator
+        public static State fromString(String s)
+        {
+            if (s == null) return null;
+            switch (s.trim().toLowerCase())
+            {
+                case "start":
+                    return START;
+                case "stop":
+                    return STOP;
+                default:
+                    throw new IllegalArgumentException("Unknown state: " + s);
+            }
+        }
+
+        @JsonProperty
+        public String toValue()
+        {
+            return name().toLowerCase();
+        }
+    }
+
+    private final State state;
+
+    /**
+     * @param state the desired operation, must be "start" or "stop"
+     */
+    @JsonCreator
+    public NodeCommandRequestPayload(
+    @JsonProperty(value = "state", required = true) String state
+    )
+    {
+        Preconditions.checkArgument(state != null && !state.isEmpty(),
+                                    "state must be provided and non-empty");
+        this.state = State.fromString(state);
+    }
+
+    /**
+     * @return the parsed enum (START or STOP)
+     */
+    @JsonProperty("state")
+    public State getState()

Review Comment:
   minor suggestion to drop `get` in line with Cassandra code style 
recommendations.
   ```suggestion
       public State state()
   ```



##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/GossipUpdateHandler.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * 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.cassandra.sidecar.handlers;
+
+import java.util.Collections;
+import java.util.Set;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.inject.Inject;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.vertx.core.http.HttpServerRequest;
+import io.vertx.core.net.SocketAddress;
+import io.vertx.ext.auth.authorization.Authorization;
+import io.vertx.ext.web.RoutingContext;
+import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions;
+import 
org.apache.cassandra.sidecar.common.request.data.NodeCommandRequestPayload;
+import org.apache.cassandra.sidecar.common.server.StorageOperations;
+import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
+import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * Handles {@code PUT /api/v1/cassandra/gossip} requests to start or stop 
Cassandra gossip.
+ *
+ * <p>Expects a JSON payload:
+ * { "state": "start" } or { "state": "stop" }
+ * and will asynchronously invoke the corresponding JMX operation.</p>
+ */
+public class GossipUpdateHandler extends NodeCommandHandler implements 
AccessProtected
+{
+    private final ExecutorPools executorPools;
+    private static final Logger LOGGER = 
LoggerFactory.getLogger(GossipUpdateHandler.class);
+
+
+    @Inject
+    public GossipUpdateHandler(InstanceMetadataFetcher metadataFetcher,
+                               ExecutorPools executorPools)
+    {
+        super(metadataFetcher, executorPools, null);
+        this.executorPools = executorPools;
+    }
+
+    @Override
+    public Set<Authorization> requiredAuthorizations()
+    {
+        return 
Collections.singleton(BasicPermissions.WRITE_GOSSIP.toAuthorization());
+    }
+
+    @Override
+    protected void handleInternal(RoutingContext context,
+                                  HttpServerRequest httpRequest,
+                                  @NotNull String host,
+                                  SocketAddress remoteAddress,
+                                  NodeCommandRequestPayload request)
+    {
+        StorageOperations storageOps = 
metadataFetcher.delegate(host).storageOperations();
+
+        executorPools.service()
+                     .runBlocking(() -> {
+                         switch (request.getState())
+                         {
+                             case START:
+                                 storageOps.startGossiping();
+                                 break;
+                             case STOP:
+                                 storageOps.stopGossiping();
+                                 break;
+                             default:
+                                 throw new IllegalStateException("Unknown 
state: " + request.getState());
+                         }
+                     })
+                     .onSuccess(ignored -> {
+                         
context.response().setStatusCode(HttpResponseStatus.ACCEPTED.code()).end();

Review Comment:
   we always respond with a JSON payload. Maybe status: OK , or status: 
Accepted are possible responses?



##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/NativeUpdateHandler.java:
##########
@@ -0,0 +1,99 @@
+/*
+ * 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.cassandra.sidecar.handlers;
+
+import java.util.Collections;
+import java.util.Set;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.inject.Inject;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.vertx.core.http.HttpServerRequest;
+import io.vertx.core.net.SocketAddress;
+import io.vertx.ext.auth.authorization.Authorization;
+import io.vertx.ext.web.RoutingContext;
+import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions;
+import 
org.apache.cassandra.sidecar.common.request.data.NodeCommandRequestPayload;
+import org.apache.cassandra.sidecar.common.server.StorageOperations;
+import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
+import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher;
+import org.jetbrains.annotations.NotNull;
+
+
+/**
+ * Handler for starting or stopping native transport on a Cassandra node.
+ * <p>
+ * Expects a JSON body:
+ * { "state": "start" }  or  { "state": "stop" }
+ * </p>
+ */
+public class NativeUpdateHandler extends NodeCommandHandler implements 
AccessProtected
+{
+    private final ExecutorPools executorPools;
+    private static final Logger LOGGER = 
LoggerFactory.getLogger(NativeUpdateHandler.class);

Review Comment:
   we can omit executorPools since it's already available in the super. LOGGER 
is unused so I suggest we remove it



##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/NativeUpdateHandler.java:
##########
@@ -0,0 +1,99 @@
+/*
+ * 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.cassandra.sidecar.handlers;
+
+import java.util.Collections;
+import java.util.Set;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.inject.Inject;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.vertx.core.http.HttpServerRequest;
+import io.vertx.core.net.SocketAddress;
+import io.vertx.ext.auth.authorization.Authorization;
+import io.vertx.ext.web.RoutingContext;
+import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions;
+import 
org.apache.cassandra.sidecar.common.request.data.NodeCommandRequestPayload;
+import org.apache.cassandra.sidecar.common.server.StorageOperations;
+import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
+import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher;
+import org.jetbrains.annotations.NotNull;
+
+
+/**
+ * Handler for starting or stopping native transport on a Cassandra node.
+ * <p>
+ * Expects a JSON body:
+ * { "state": "start" }  or  { "state": "stop" }
+ * </p>
+ */
+public class NativeUpdateHandler extends NodeCommandHandler implements 
AccessProtected

Review Comment:
   can we annotate with singleton?



##########
server/src/main/java/org/apache/cassandra/sidecar/modules/multibindings/VertxRouteMapKeys.java:
##########
@@ -253,4 +253,14 @@ interface UpdateServiceConfigurationRouteKey extends 
RouteClassKey
         HttpMethod HTTP_METHOD = HttpMethod.PUT;
         String ROUTE_URI = ApiEndpointsV1.SERVICE_CONFIG_ROUTE;
     }
+    interface UpdateNodeGossipRouteKey extends RouteClassKey
+    {
+        HttpMethod HTTP_METHOD = HttpMethod.PUT;
+        String    ROUTE_URI   = ApiEndpointsV1.GOSSIP_ROUTE;
+    }
+    interface UpdateNodeNativeRouteKey extends RouteClassKey

Review Comment:
   NIT
   ```suggestion
       interface UpdateNodeNativeStateRouteKey extends RouteClassKey
   ```



##########
server/src/main/java/org/apache/cassandra/sidecar/modules/multibindings/VertxRouteMapKeys.java:
##########
@@ -253,4 +253,14 @@ interface UpdateServiceConfigurationRouteKey extends 
RouteClassKey
         HttpMethod HTTP_METHOD = HttpMethod.PUT;
         String ROUTE_URI = ApiEndpointsV1.SERVICE_CONFIG_ROUTE;
     }
+    interface UpdateNodeGossipRouteKey extends RouteClassKey

Review Comment:
   NIT
   ```suggestion
       interface UpdateNodeGossipStateRouteKey extends RouteClassKey
   ```



##########
server/src/test/java/org/apache/cassandra/sidecar/handlers/NativeUpdateHandlerTest.java:
##########
@@ -0,0 +1,153 @@
+/*
+ * 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.cassandra.sidecar.handlers;
+
+import java.util.Collections;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.Module;
+import com.google.inject.Provides;
+import com.google.inject.Singleton;
+import com.google.inject.util.Modules;
+import io.vertx.core.Vertx;
+import io.vertx.ext.web.client.WebClient;
+import io.vertx.ext.web.client.predicate.ResponsePredicate;
+import io.vertx.junit5.VertxExtension;
+import io.vertx.junit5.VertxTestContext;
+import org.apache.cassandra.sidecar.TestModule;
+import org.apache.cassandra.sidecar.cluster.CassandraAdapterDelegate;
+import org.apache.cassandra.sidecar.cluster.InstancesMetadata;
+import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata;
+import org.apache.cassandra.sidecar.common.server.StorageOperations;
+import org.apache.cassandra.sidecar.modules.SidecarModules;
+import org.apache.cassandra.sidecar.server.Server;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+/**
+ * Test for {@link NativeUpdateHandler}
+ */
+@ExtendWith(VertxExtension.class)
+public class NativeUpdateHandlerTest
+{
+    static final Logger LOGGER = 
LoggerFactory.getLogger(GossipUpdateHandlerTest.class);
+    Vertx vertx;
+    Server server;
+    StorageOperations mockStorageOperations = mock(StorageOperations.class);
+
+    @BeforeEach
+    void before() throws InterruptedException
+    {
+        Injector injector;
+        Module testOverride = Modules.override(new TestModule()).with(new 
NativeUpdateHandlerTest.NativeUpdateHandlerTestModule());
+        injector = 
Guice.createInjector(Modules.override(SidecarModules.all()).with(testOverride));
+        vertx = injector.getInstance(Vertx.class);
+        server = injector.getInstance(Server.class);
+        VertxTestContext context = new VertxTestContext();
+        server.start().onSuccess(s -> 
context.completeNow()).onFailure(context::failNow);
+        context.awaitCompletion(5, TimeUnit.SECONDS);
+    }
+
+    @AfterEach
+    void after() throws InterruptedException
+    {
+        CountDownLatch closeLatch = new CountDownLatch(1);
+        server.close().onSuccess(res -> closeLatch.countDown());
+        if (closeLatch.await(60, TimeUnit.SECONDS)) LOGGER.info("Close event 
received before timeout.");
+        else LOGGER.error("Close event timed out.");
+    }
+
+    @Test
+    void testStartNative(VertxTestContext ctx)
+    {
+        WebClient client = WebClient.create(vertx);
+        String payload = "{\"state\":\"start\"}";
+        client.put(server.actualPort(), "127.0.0.1", 
"/api/v1/cassandra/native").expect(ResponsePredicate.SC_ACCEPTED).sendBuffer(io.vertx.core.buffer.Buffer.buffer(payload),
 ctx.succeeding(resp -> {
+            verify(mockStorageOperations, times(1)).startNativeTransport();
+            ctx.completeNow();
+        }));
+    }
+
+    @Test
+    void testStopNative(VertxTestContext ctx)
+    {
+        WebClient client = WebClient.create(vertx);
+        String payload = "{\"state\":\"stop\"}";
+        client.put(server.actualPort(), "127.0.0.1", 
"/api/v1/cassandra/native").expect(ResponsePredicate.SC_ACCEPTED).sendBuffer(io.vertx.core.buffer.Buffer.buffer(payload),
 ctx.succeeding(resp -> {
+            verify(mockStorageOperations, times(1)).stopNativeTransport();
+            ctx.completeNow();
+        }));
+    }
+
+    @Test
+    void testInvalidState(VertxTestContext ctx)
+    {
+        WebClient client = WebClient.create(vertx);
+        String payload = "{\"state\":\"foo\"}";
+        client.put(server.actualPort(), "127.0.0.1", 
"/api/v1/cassandra/native").expect(ResponsePredicate.SC_BAD_REQUEST).sendBuffer(io.vertx.core.buffer.Buffer.buffer(payload),
 ctx.succeeding(resp -> {
+            ctx.completeNow();
+        }));
+    }
+
+
+    /**
+     * Test guice module for Node Decommission handler tests
+     */
+    class NativeUpdateHandlerTestModule extends AbstractModule
+    {
+        @Provides
+        @Singleton
+        public InstancesMetadata instanceMetadata()
+        {
+            final int instanceId = 100;
+            final String host = "127.0.0.1";
+            final InstanceMetadata instanceMetadata = 
mock(InstanceMetadata.class);

Review Comment:
   NIT, omit final modifier for local fields
   ```suggestion
               int instanceId = 100;
               String host = "127.0.0.1";
               InstanceMetadata instanceMetadata = mock(InstanceMetadata.class);
   ```



##########
client-common/src/main/java/org/apache/cassandra/sidecar/common/request/data/NodeCommandRequestPayload.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.cassandra.sidecar.common.request.data;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.cassandra.sidecar.common.utils.Preconditions;
+
+/**
+ * Request payload for start/stop operations (gossip, native transport, etc.).
+ *
+ * <p>Valid JSON:</p>
+ * <pre>
+ *   { "state": "start" }
+ *   { "state": "stop"  }
+ * </pre>
+ */
+@JsonInclude(JsonInclude.Include.NON_NULL)
+public class NodeCommandRequestPayload
+{
+    /**
+     * Node Command State
+     */
+    public enum State
+    {
+        START, STOP;
+
+        @JsonCreator
+        public static State fromString(String s)
+        {
+            if (s == null) return null;

Review Comment:
   should we throw here instead? 



##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/GossipUpdateHandler.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * 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.cassandra.sidecar.handlers;
+
+import java.util.Collections;
+import java.util.Set;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.inject.Inject;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.vertx.core.http.HttpServerRequest;
+import io.vertx.core.net.SocketAddress;
+import io.vertx.ext.auth.authorization.Authorization;
+import io.vertx.ext.web.RoutingContext;
+import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions;
+import 
org.apache.cassandra.sidecar.common.request.data.NodeCommandRequestPayload;
+import org.apache.cassandra.sidecar.common.server.StorageOperations;
+import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
+import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * Handles {@code PUT /api/v1/cassandra/gossip} requests to start or stop 
Cassandra gossip.
+ *
+ * <p>Expects a JSON payload:
+ * { "state": "start" } or { "state": "stop" }
+ * and will asynchronously invoke the corresponding JMX operation.</p>
+ */
+public class GossipUpdateHandler extends NodeCommandHandler implements 
AccessProtected
+{
+    private final ExecutorPools executorPools;
+    private static final Logger LOGGER = 
LoggerFactory.getLogger(GossipUpdateHandler.class);
+
+
+    @Inject
+    public GossipUpdateHandler(InstanceMetadataFetcher metadataFetcher,
+                               ExecutorPools executorPools)
+    {
+        super(metadataFetcher, executorPools, null);
+        this.executorPools = executorPools;

Review Comment:
   this can be omitted



##########
client/src/main/java/org/apache/cassandra/sidecar/client/RequestContext.java:
##########


Review Comment:
   We also need to expose the new methods via the client 
(`org.apache.cassandra.sidecar.client.SidecarClient`)



##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/GossipUpdateHandler.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * 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.cassandra.sidecar.handlers;
+
+import java.util.Collections;
+import java.util.Set;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.inject.Inject;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.vertx.core.http.HttpServerRequest;
+import io.vertx.core.net.SocketAddress;
+import io.vertx.ext.auth.authorization.Authorization;
+import io.vertx.ext.web.RoutingContext;
+import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions;
+import 
org.apache.cassandra.sidecar.common.request.data.NodeCommandRequestPayload;
+import org.apache.cassandra.sidecar.common.server.StorageOperations;
+import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
+import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * Handles {@code PUT /api/v1/cassandra/gossip} requests to start or stop 
Cassandra gossip.
+ *
+ * <p>Expects a JSON payload:
+ * { "state": "start" } or { "state": "stop" }
+ * and will asynchronously invoke the corresponding JMX operation.</p>
+ */
+public class GossipUpdateHandler extends NodeCommandHandler implements 
AccessProtected
+{
+    private final ExecutorPools executorPools;
+    private static final Logger LOGGER = 
LoggerFactory.getLogger(GossipUpdateHandler.class);

Review Comment:
   executorPools is already available on the super class, so can be omitted. 
LOGGER is unused so it should be removed unless it is needed



##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/GossipUpdateHandler.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * 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.cassandra.sidecar.handlers;
+
+import java.util.Collections;
+import java.util.Set;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.inject.Inject;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.vertx.core.http.HttpServerRequest;
+import io.vertx.core.net.SocketAddress;
+import io.vertx.ext.auth.authorization.Authorization;
+import io.vertx.ext.web.RoutingContext;
+import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions;
+import 
org.apache.cassandra.sidecar.common.request.data.NodeCommandRequestPayload;
+import org.apache.cassandra.sidecar.common.server.StorageOperations;
+import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
+import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * Handles {@code PUT /api/v1/cassandra/gossip} requests to start or stop 
Cassandra gossip.
+ *
+ * <p>Expects a JSON payload:
+ * { "state": "start" } or { "state": "stop" }
+ * and will asynchronously invoke the corresponding JMX operation.</p>
+ */
+public class GossipUpdateHandler extends NodeCommandHandler implements 
AccessProtected

Review Comment:
   we should probably annotate as singleton



##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/NativeUpdateHandler.java:
##########
@@ -0,0 +1,99 @@
+/*
+ * 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.cassandra.sidecar.handlers;
+
+import java.util.Collections;
+import java.util.Set;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.inject.Inject;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.vertx.core.http.HttpServerRequest;
+import io.vertx.core.net.SocketAddress;
+import io.vertx.ext.auth.authorization.Authorization;
+import io.vertx.ext.web.RoutingContext;
+import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions;
+import 
org.apache.cassandra.sidecar.common.request.data.NodeCommandRequestPayload;
+import org.apache.cassandra.sidecar.common.server.StorageOperations;
+import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
+import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher;
+import org.jetbrains.annotations.NotNull;
+
+
+/**
+ * Handler for starting or stopping native transport on a Cassandra node.
+ * <p>
+ * Expects a JSON body:
+ * { "state": "start" }  or  { "state": "stop" }
+ * </p>
+ */
+public class NativeUpdateHandler extends NodeCommandHandler implements 
AccessProtected
+{
+    private final ExecutorPools executorPools;
+    private static final Logger LOGGER = 
LoggerFactory.getLogger(NativeUpdateHandler.class);
+
+
+    @Inject
+    public NativeUpdateHandler(InstanceMetadataFetcher metadataFetcher,
+                               ExecutorPools executorPools)
+    {
+        super(metadataFetcher, executorPools, null);
+        this.executorPools = executorPools;
+    }
+
+    @Override
+    public Set<Authorization> requiredAuthorizations()
+    {
+        return 
Collections.singleton(BasicPermissions.WRITE_NATIVE.toAuthorization());
+    }
+
+    @Override
+    protected void handleInternal(RoutingContext context,
+                                  HttpServerRequest httpRequest,
+                                  @NotNull String host,
+                                  SocketAddress remoteAddress,
+                                  NodeCommandRequestPayload request)
+    {
+        StorageOperations storageOps = 
metadataFetcher.delegate(host).storageOperations();
+
+        executorPools.service()
+                     .runBlocking(() -> {
+                         switch (request.getState())
+                         {
+                             case START:
+                                 storageOps.startNativeTransport();
+                                 break;
+                             case STOP:
+                                 storageOps.stopNativeTransport();
+                                 break;
+                             default:
+                                 throw new IllegalStateException("Unknown 
state: " + request.getState());
+                         }
+                     })
+                     .onSuccess(ignored -> {
+                         
context.response().setStatusCode(HttpResponseStatus.ACCEPTED.code()).end();

Review Comment:
   let's respond with a json payload, status: OK is acceptable, or something 
along those lines 



##########
server/src/main/java/org/apache/cassandra/sidecar/modules/CassandraOperationsModule.java:
##########
@@ -173,4 +175,26 @@ VertxRoute tableStatsRoute(RouteBuilder.Factory factory,
                       .handler(validateTableExistenceHandler)
                       .handler(tableStatsHandler).build();
     }
+
+    @ProvidesIntoMap
+    @KeyClassMapKey(VertxRouteMapKeys.UpdateNodeGossipRouteKey.class)
+    VertxRoute cassandraGossipRoute(RouteBuilder.Factory factory,

Review Comment:
   NIT
   ```suggestion
       VertxRoute cassandraChangeGossipStateRoute(RouteBuilder.Factory factory,
   ```



##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/NativeUpdateHandler.java:
##########
@@ -0,0 +1,99 @@
+/*
+ * 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.cassandra.sidecar.handlers;
+
+import java.util.Collections;
+import java.util.Set;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.inject.Inject;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.vertx.core.http.HttpServerRequest;
+import io.vertx.core.net.SocketAddress;
+import io.vertx.ext.auth.authorization.Authorization;
+import io.vertx.ext.web.RoutingContext;
+import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions;
+import 
org.apache.cassandra.sidecar.common.request.data.NodeCommandRequestPayload;
+import org.apache.cassandra.sidecar.common.server.StorageOperations;
+import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
+import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher;
+import org.jetbrains.annotations.NotNull;
+
+
+/**
+ * Handler for starting or stopping native transport on a Cassandra node.
+ * <p>
+ * Expects a JSON body:
+ * { "state": "start" }  or  { "state": "stop" }
+ * </p>
+ */
+public class NativeUpdateHandler extends NodeCommandHandler implements 
AccessProtected
+{
+    private final ExecutorPools executorPools;
+    private static final Logger LOGGER = 
LoggerFactory.getLogger(NativeUpdateHandler.class);
+
+
+    @Inject
+    public NativeUpdateHandler(InstanceMetadataFetcher metadataFetcher,
+                               ExecutorPools executorPools)
+    {
+        super(metadataFetcher, executorPools, null);
+        this.executorPools = executorPools;

Review Comment:
   should be omitted



##########
server/src/main/java/org/apache/cassandra/sidecar/modules/CassandraOperationsModule.java:
##########
@@ -173,4 +175,26 @@ VertxRoute tableStatsRoute(RouteBuilder.Factory factory,
                       .handler(validateTableExistenceHandler)
                       .handler(tableStatsHandler).build();
     }
+
+    @ProvidesIntoMap
+    @KeyClassMapKey(VertxRouteMapKeys.UpdateNodeGossipRouteKey.class)
+    VertxRoute cassandraGossipRoute(RouteBuilder.Factory factory,
+                                    GossipUpdateHandler nodeGossipHandler)
+    {
+        return factory.builderForRoute()
+                      .setBodyHandler(true)
+                      .handler(nodeGossipHandler)
+                      .build();
+    }
+
+    @ProvidesIntoMap
+    @KeyClassMapKey(VertxRouteMapKeys.UpdateNodeNativeRouteKey.class)
+    VertxRoute cassandraNativeRoute(RouteBuilder.Factory factory,

Review Comment:
   NIT
   ```suggestion
       VertxRoute cassandraChangeNativeStateRoute(RouteBuilder.Factory factory,
   ```



##########
server/src/test/java/org/apache/cassandra/sidecar/handlers/NativeUpdateHandlerTest.java:
##########
@@ -0,0 +1,153 @@
+/*
+ * 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.cassandra.sidecar.handlers;
+
+import java.util.Collections;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.Module;
+import com.google.inject.Provides;
+import com.google.inject.Singleton;
+import com.google.inject.util.Modules;
+import io.vertx.core.Vertx;
+import io.vertx.ext.web.client.WebClient;
+import io.vertx.ext.web.client.predicate.ResponsePredicate;
+import io.vertx.junit5.VertxExtension;
+import io.vertx.junit5.VertxTestContext;
+import org.apache.cassandra.sidecar.TestModule;
+import org.apache.cassandra.sidecar.cluster.CassandraAdapterDelegate;
+import org.apache.cassandra.sidecar.cluster.InstancesMetadata;
+import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata;
+import org.apache.cassandra.sidecar.common.server.StorageOperations;
+import org.apache.cassandra.sidecar.modules.SidecarModules;
+import org.apache.cassandra.sidecar.server.Server;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+/**
+ * Test for {@link NativeUpdateHandler}
+ */
+@ExtendWith(VertxExtension.class)
+public class NativeUpdateHandlerTest
+{
+    static final Logger LOGGER = 
LoggerFactory.getLogger(GossipUpdateHandlerTest.class);

Review Comment:
   Do we want this to be the `NativeUpdateHandlerTest` class instead?
   ```suggestion
       static final Logger LOGGER = 
LoggerFactory.getLogger(NativeUpdateHandlerTest.class);
   ```



-- 
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: pr-unsubscr...@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org


Reply via email to