sarankk commented on code in PR #200:
URL: https://github.com/apache/cassandra-sidecar/pull/200#discussion_r1974392153


##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/RingHandler.java:
##########
@@ -16,7 +16,7 @@
  * limitations under the License.
  */
 
-package org.apache.cassandra.sidecar.routes;
+package org.apache.cassandra.sidecar.handlers;

Review Comment:
   My only concern with this package name change is, currently we have just API 
route specific handlers here, other speciality handlers in their respective 
packages, which keeps handlers more organized. Naming it generic, might lead to 
all handlers being put here. How about moving `handlers` package under `routes` 
package? 



##########
server/src/main/java/org/apache/cassandra/sidecar/db/schema/SidecarSchemaInitializer.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * 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.db.schema;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.datastax.driver.core.Session;
+import io.vertx.core.Promise;
+import io.vertx.core.Vertx;
+import org.apache.cassandra.sidecar.common.server.CQLSessionProvider;
+import org.apache.cassandra.sidecar.common.server.utils.DurationSpec;
+import 
org.apache.cassandra.sidecar.common.server.utils.MillisecondBoundConfiguration;
+import org.apache.cassandra.sidecar.config.SidecarConfiguration;
+import org.apache.cassandra.sidecar.coordination.ClusterLease;
+import 
org.apache.cassandra.sidecar.coordination.ExecuteOnClusterLeaseholderOnly;
+import org.apache.cassandra.sidecar.exceptions.CassandraUnavailableException;
+import 
org.apache.cassandra.sidecar.exceptions.SidecarSchemaModificationException;
+import org.apache.cassandra.sidecar.metrics.SchemaMetrics;
+import org.apache.cassandra.sidecar.tasks.PeriodicTask;
+import org.apache.cassandra.sidecar.tasks.PeriodicTaskExecutor;
+import org.apache.cassandra.sidecar.tasks.ScheduleDecision;
+
+import static 
org.apache.cassandra.sidecar.server.SidecarServerEvents.ON_CASSANDRA_CQL_READY;
+import static 
org.apache.cassandra.sidecar.server.SidecarServerEvents.ON_SIDECAR_SCHEMA_INITIALIZED;
+
+/**
+ * Initializer that retries until sidecar schema is initialized. Once 
initialized, it un-schedules itself.
+ */
+public class SidecarSchemaInitializer implements PeriodicTask
+{
+    private static final Logger LOGGER = 
LoggerFactory.getLogger(SidecarSchemaInitializer.class);
+    private static final DurationSpec INITIALIZATION_LOOP_DELAY = 
MillisecondBoundConfiguration.parse("1s");
+
+    private final CQLSessionProvider cqlSessionProvider;
+    private final SidecarInternalKeyspace sidecarInternalKeyspace;
+    private final SchemaMetrics schemaMetrics;
+    private final ClusterLease clusterLease;
+    private final boolean isSidecarSchemaEnabled;
+    private Vertx vertx;
+    private PeriodicTaskExecutor periodicTaskExecutor;
+
+    public SidecarSchemaInitializer(SidecarConfiguration sidecarConfiguration,
+                                    CQLSessionProvider cqlSessionProvider,
+                                    SidecarInternalKeyspace 
sidecarInternalKeyspace,
+                                    SchemaMetrics schemaMetrics,
+                                    ClusterLease clusterLease)
+    {
+        this.isSidecarSchemaEnabled = 
sidecarConfiguration.serviceConfiguration().schemaKeyspaceConfiguration().isEnabled();
+        this.cqlSessionProvider = cqlSessionProvider;
+        this.sidecarInternalKeyspace = sidecarInternalKeyspace;
+        this.schemaMetrics = schemaMetrics;
+        this.clusterLease = clusterLease;
+    }
+
+    @Override
+    public void deploy(Vertx vertx, PeriodicTaskExecutor executor)
+    {
+        if (!isSidecarSchemaEnabled)
+        {
+            LOGGER.info("SidecarSchema is disabled. Skip deployment of 
SidecarSchemaInitializer");
+            return;
+        }
+        this.vertx = vertx;
+        this.periodicTaskExecutor = executor;
+        // deploy when CQL connection is established
+        vertx.eventBus().localConsumer(ON_CASSANDRA_CQL_READY.address(),
+                                       ignored -> 
PeriodicTask.super.deploy(vertx, executor));
+    }
+
+    @Override
+    public ScheduleDecision scheduleDecision()
+    {
+        if (cqlSessionProvider.getIfConnected() == null)
+        {
+            LOGGER.debug("CQL connection is not yet established. Skip this run 
of initialization.");
+            return ScheduleDecision.SKIP;
+        }
+        return ScheduleDecision.EXECUTE;
+    }
+
+    @Override
+    public DurationSpec delay()
+    {
+        return INITIALIZATION_LOOP_DELAY;
+    }
+
+    @Override
+    public void execute(Promise<Void> promise)
+    {
+        try
+        {
+            Session session = cqlSessionProvider.get();
+            boolean isInitialized = 
sidecarInternalKeyspace.initialize(session, this::shouldCreateSchema);
+
+            if (isInitialized)
+            {
+                LOGGER.info("Sidecar schema is initialized. Stopping 
SchemaSidecarInitializer");
+                periodicTaskExecutor.unschedule(this);
+                reportSidecarSchemaInitialized();
+            }
+        }
+        catch (Exception ex)
+        {
+            LOGGER.warn("Failed to initialize schema. Retry in {}", delay(), 
ex);
+            if (ex instanceof CassandraUnavailableException) // not quite 
expected here according to the schedule decision, but still check for it
+            {
+                return; // do not count Cassandra unavailable as failure
+            }
+            else if (ex instanceof SidecarSchemaModificationException)
+            {
+                LOGGER.warn("Failed to modify schema", ex);
+                schemaMetrics.failedModifications.metric.update(1);

Review Comment:
    Since we are initializing, we should use `failedInitializations` here?



##########
server/src/main/java/org/apache/cassandra/sidecar/modules/multibindings/RouteClassKey.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.modules.multibindings;
+
+import io.vertx.core.http.HttpMethod;
+
+/**
+ * Used for defining the class key for VertxRoute multi-binding map.
+ * <p>The RouteClassKey also defines the contract on the existence of static 
fields in its implementations.
+ * The required static fields are read out using reflection. Below is the list:
+ * <ul>
+ *     <li>{@code HttpMethod HTTP_METHOD}</li>
+ *     <li>{@code String ROUTE_URI}</li>
+ * </ul>
+ */
+public interface RouteClassKey extends ClassKey
+{
+    String HTTP_METHOD_FIELD_NAME = "HTTP_METHOD";
+    String ROUTE_URI_FIELD_NAME = "ROUTE_URI";
+
+    /**
+     * Reads the static {@link HttpMethod} defined in the class
+     *
+     * @param classKey class to read
+     * @return http method
+     */
+    static HttpMethod httpMethod(Class<? extends RouteClassKey> classKey)
+    {
+        try
+        {
+            return (HttpMethod) 
classKey.getDeclaredField(HTTP_METHOD_FIELD_NAME).get(null);
+        }
+        catch (IllegalAccessException | NoSuchFieldException | 
ClassCastException e)
+        {
+            throw new RuntimeException(e);
+        }
+    }
+

Review Comment:
   Nit: missing javadoc



##########
server/src/main/java/org/apache/cassandra/sidecar/modules/HealthCheckModule.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.modules;
+
+import java.util.Collections;
+import java.util.Map;
+
+import com.google.inject.AbstractModule;
+import com.google.inject.multibindings.ProvidesIntoMap;
+import org.apache.cassandra.sidecar.cluster.InstancesMetadata;
+import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
+import org.apache.cassandra.sidecar.config.SidecarConfiguration;
+import org.apache.cassandra.sidecar.handlers.CassandraHealthHandler;
+import org.apache.cassandra.sidecar.handlers.GossipHealthHandler;
+import org.apache.cassandra.sidecar.handlers.RouteBuilder;
+import org.apache.cassandra.sidecar.metrics.SidecarMetrics;
+import org.apache.cassandra.sidecar.modules.multibindings.KeyClassMapKey;
+import org.apache.cassandra.sidecar.modules.multibindings.PeriodicTaskMapKeys;
+import org.apache.cassandra.sidecar.modules.multibindings.VertxRouteMapKeys;
+import org.apache.cassandra.sidecar.routes.VertxRoute;
+import org.apache.cassandra.sidecar.tasks.HealthCheckPeriodicTask;
+import org.apache.cassandra.sidecar.tasks.PeriodicTask;
+
+/**
+ * Provides the capability to perform health checks on various components in 
both Sidecar and Cassandra
+ */
+public class HealthCheckModule extends AbstractModule
+{
+    public static final Map<String, String> OK_STATUS = 
Collections.singletonMap("status", "OK");
+    public static final Map<String, String> NOT_OK_STATUS = 
Collections.singletonMap("status", "NOT_OK");
+
+    @ProvidesIntoMap
+    @KeyClassMapKey(PeriodicTaskMapKeys.HealthCheckPeriodicTaskKey.class)
+    PeriodicTask healthCheckPeriodicTask(SidecarConfiguration configuration,
+                                         InstancesMetadata instancesMetadata,
+                                         ExecutorPools executorPools,
+                                         SidecarMetrics metrics)
+    {
+        return new HealthCheckPeriodicTask(configuration, instancesMetadata, 
executorPools, metrics);
+    }
+
+    @ProvidesIntoMap
+    @KeyClassMapKey(VertxRouteMapKeys.SidecarHealthRouteKey.class)
+    VertxRoute sidecarHealthRoute(RouteBuilder.Factory factory)
+    {
+        return factory.builderForUnprotectedRoute()
+                      .handler(context -> context.json(OK_STATUS))
+                      .build();
+    }
+
+    @Deprecated
+    @ProvidesIntoMap
+    @KeyClassMapKey(VertxRouteMapKeys.CassandraHealthRouteKey.class)
+    VertxRoute cassandraHealthRoute(RouteBuilder.Factory factory,
+                                    CassandraHealthHandler 
cassandraHealthHandler)
+    {
+        // Backwards compatibility for the Cassandra health endpoint
+        return factory.builderForUnprotectedRoute()

Review Comment:
   Nit: the phrase `Unprotected` reads like it is unauthenticated and 
unauthorized.  Whereas these endpoints are only unauthorized, they are still 
authenticated. Should we rename to `builderForUnauthorizedRoute` ? 



##########
server/src/main/java/org/apache/cassandra/sidecar/modules/multibindings/RouteClassKey.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.modules.multibindings;
+
+import io.vertx.core.http.HttpMethod;
+
+/**
+ * Used for defining the class key for VertxRoute multi-binding map.
+ * <p>The RouteClassKey also defines the contract on the existence of static 
fields in its implementations.
+ * The required static fields are read out using reflection. Below is the list:
+ * <ul>
+ *     <li>{@code HttpMethod HTTP_METHOD}</li>
+ *     <li>{@code String ROUTE_URI}</li>
+ * </ul>
+ */
+public interface RouteClassKey extends ClassKey
+{
+    String HTTP_METHOD_FIELD_NAME = "HTTP_METHOD";
+    String ROUTE_URI_FIELD_NAME = "ROUTE_URI";
+
+    /**
+     * Reads the static {@link HttpMethod} defined in the class
+     *
+     * @param classKey class to read
+     * @return http method
+     */
+    static HttpMethod httpMethod(Class<? extends RouteClassKey> classKey)
+    {
+        try
+        {
+            return (HttpMethod) 
classKey.getDeclaredField(HTTP_METHOD_FIELD_NAME).get(null);

Review Comment:
   Instead of static members can we have methods for `httpMethod` and 
`routeURI` in `RouteClassKey`? to make sure all `RouteClassKey` will have these 
2 set. Currently `RouteClassKey.httpMethod` can throw `NoSuchFieldException` if 
we forget to set `HTTP_METHOD` 



##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/CassandraHealthHandler.java:
##########
@@ -50,7 +50,7 @@ public class CassandraHealthHandler extends 
AbstractHandler<Void>
      * @param validator       a validator instance to validate 
Cassandra-specific input
      */
     @Inject
-    protected CassandraHealthHandler(InstanceMetadataFetcher metadataFetcher,
+    public CassandraHealthHandler(InstanceMetadataFetcher metadataFetcher,

Review Comment:
   Nit: Incorrect indentation 



##########
server/src/main/java/org/apache/cassandra/sidecar/routes/VertxRoute.java:
##########
@@ -0,0 +1,50 @@
+/*
+ * 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.routes;
+
+import java.util.function.Consumer;
+
+import io.vertx.ext.web.Router;
+
+/**
+ * Defines a vert route.

Review Comment:
   Missing spelling. `Defines a Vert.x route`



##########
server/src/main/java/org/apache/cassandra/sidecar/db/RestoreSliceDatabaseAccessor.java:
##########
@@ -42,10 +42,9 @@ public class RestoreSliceDatabaseAccessor extends 
DatabaseAccessor<RestoreSlices
 
     @Inject
     protected RestoreSliceDatabaseAccessor(SidecarSchema sidecarSchema,
-                                           RestoreSlicesSchema 
restoreSlicesSchema,
                                            CQLSessionProvider 
cqlSessionProvider)
     {
-        super(restoreSlicesSchema, cqlSessionProvider);
+        super(sidecarSchema.tableSchema(RestoreSlicesSchema.class), 
cqlSessionProvider);

Review Comment:
   Nit: should we call this method `tableSchemaByClass` ? 



##########
server/src/main/java/org/apache/cassandra/sidecar/modules/CassandraOperationsModule.java:
##########
@@ -0,0 +1,189 @@
+/*
+ * 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.modules;
+
+import com.google.inject.AbstractModule;
+import com.google.inject.multibindings.ProvidesIntoMap;
+import org.apache.cassandra.sidecar.handlers.ConnectedClientStatsHandler;
+import org.apache.cassandra.sidecar.handlers.GossipInfoHandler;
+import org.apache.cassandra.sidecar.handlers.KeyspaceRingHandler;
+import org.apache.cassandra.sidecar.handlers.KeyspaceSchemaHandler;
+import org.apache.cassandra.sidecar.handlers.ListOperationalJobsHandler;
+import org.apache.cassandra.sidecar.handlers.NodeDecommissionHandler;
+import org.apache.cassandra.sidecar.handlers.OperationalJobHandler;
+import org.apache.cassandra.sidecar.handlers.RingHandler;
+import org.apache.cassandra.sidecar.handlers.RouteBuilder;
+import org.apache.cassandra.sidecar.handlers.SchemaHandler;
+import org.apache.cassandra.sidecar.handlers.StreamStatsHandler;
+import org.apache.cassandra.sidecar.handlers.TokenRangeReplicaMapHandler;
+import org.apache.cassandra.sidecar.handlers.cassandra.NodeSettingsHandler;
+import org.apache.cassandra.sidecar.modules.multibindings.KeyClassMapKey;
+import org.apache.cassandra.sidecar.modules.multibindings.VertxRouteMapKeys;
+import org.apache.cassandra.sidecar.routes.VertxRoute;
+
+/**
+ * Provides the capability to query and invoke Cassandra operations
+ */
+public class CassandraOperationsModule extends AbstractModule
+{
+    @ProvidesIntoMap
+    
@KeyClassMapKey(VertxRouteMapKeys.CassandraConnectedClientStatsRouteKey.class)
+    VertxRoute cassandraConnectedClientStatsRoute(RouteBuilder.Factory factory,
+                                                  ConnectedClientStatsHandler 
connectedClientStatsHandler)
+    {
+        return factory.builderForRoute()
+                      .handler(connectedClientStatsHandler)
+                      .build();
+    }
+
+    @ProvidesIntoMap
+    @KeyClassMapKey(VertxRouteMapKeys.CassandraOperationalJobRouteKey.class)
+    VertxRoute cassandraOperationalJobRoute(RouteBuilder.Factory factory,
+                                            OperationalJobHandler 
operationalJobHandler)
+    {
+        return factory.builderForRoute()
+                      .handler(operationalJobHandler)
+                      .build();
+    }
+
+    @ProvidesIntoMap
+    
@KeyClassMapKey(VertxRouteMapKeys.ListCassandraOperationalJobRouteKey.class)
+    VertxRoute listCassandraOperationalJobRoute(RouteBuilder.Factory factory,
+                                                ListOperationalJobsHandler 
listOperationalJobsHandler)
+    {
+        return factory.builderForRoute()
+                      .handler(listOperationalJobsHandler)
+                      .build();
+    }
+
+    @ProvidesIntoMap
+    @KeyClassMapKey(VertxRouteMapKeys.CassandraNodeDecommissionRouteKey.class)
+    VertxRoute cassandraNodeDecommissionRoute(RouteBuilder.Factory factory,
+                                              NodeDecommissionHandler 
nodeDecommissionHandler)
+    {
+        return factory.builderForRoute()
+                      .handler(nodeDecommissionHandler)
+                      .build();
+    }
+
+    @ProvidesIntoMap
+    @KeyClassMapKey(VertxRouteMapKeys.CassandraStreamStatsRouteKey.class)
+    VertxRoute cassandraStreamStatsRoute(RouteBuilder.Factory factory,
+                                         StreamStatsHandler streamStatsHandler)
+    {
+        return factory.builderForRoute()
+                      .handler(streamStatsHandler)
+                      .build();
+    }
+
+    @ProvidesIntoMap
+    @KeyClassMapKey(VertxRouteMapKeys.CassandraNodeSettingsRouteKey.class)
+    VertxRoute cassandraNodeSettings(RouteBuilder.Factory factory,
+                                     NodeSettingsHandler nodeSettingsHandler)
+    {
+        // Node settings endpoint is not Access protected. Any user who can 
log in into Cassandra is able to view
+        // node settings information. Since sidecar and cassandra share list 
of authenticated identities, sidecar's
+        // authenticated users can also read node settings information.
+        return factory.builderForUnprotectedRoute()
+                      .handler(nodeSettingsHandler)
+                      .build();
+    }
+
+    @ProvidesIntoMap
+    @KeyClassMapKey(VertxRouteMapKeys.AllKeyspacesSchemaRouteKey.class)
+    VertxRoute cassandraSchemaRoute(RouteBuilder.Factory factory,
+                                    SchemaHandler schemaHandler)
+    {
+        return factory.builderForRoute()

Review Comment:
   Nit: Getting schema information doesn't seem like a operation related. We 
can leave it as is for now, but later we can move it to `CassandraModule` or 
`CassandraInfoModule`?. Same with ring, token, gossip endpoint



##########
server/src/main/java/org/apache/cassandra/sidecar/modules/ApiModule.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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.modules;
+
+import com.google.inject.AbstractModule;
+import com.google.inject.Provides;
+import com.google.inject.Singleton;
+import com.google.inject.multibindings.ProvidesIntoMap;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.vertx.core.Vertx;
+import io.vertx.core.VertxOptions;
+import io.vertx.core.file.FileSystemOptions;
+import io.vertx.ext.dropwizard.DropwizardMetricsOptions;
+import io.vertx.ext.dropwizard.Match;
+import io.vertx.ext.dropwizard.MatchType;
+import io.vertx.ext.web.Router;
+import io.vertx.ext.web.handler.ErrorHandler;
+import io.vertx.ext.web.handler.LoggerHandler;
+import io.vertx.ext.web.handler.TimeoutHandler;
+import org.apache.cassandra.sidecar.common.ApiEndpointsV1;
+import org.apache.cassandra.sidecar.config.FileSystemOptionsConfiguration;
+import org.apache.cassandra.sidecar.config.SidecarConfiguration;
+import org.apache.cassandra.sidecar.config.VertxConfiguration;
+import org.apache.cassandra.sidecar.config.VertxMetricsConfiguration;
+import org.apache.cassandra.sidecar.handlers.JsonErrorHandler;
+import org.apache.cassandra.sidecar.handlers.RouteBuilder;
+import org.apache.cassandra.sidecar.handlers.RoutingOrder;
+import org.apache.cassandra.sidecar.handlers.TimeSkewHandler;
+import org.apache.cassandra.sidecar.logging.SidecarLoggerHandler;
+import org.apache.cassandra.sidecar.metrics.MetricRegistryFactory;
+import org.apache.cassandra.sidecar.modules.multibindings.KeyClassMapKey;
+import 
org.apache.cassandra.sidecar.modules.multibindings.MultiBindingTypeResolver;
+import org.apache.cassandra.sidecar.modules.multibindings.RouteClassKey;
+import org.apache.cassandra.sidecar.modules.multibindings.VertxRouteMapKeys;
+import org.apache.cassandra.sidecar.routes.SettableVertxRoute;
+import org.apache.cassandra.sidecar.routes.VertxRoute;
+
+import static 
org.apache.cassandra.sidecar.common.ApiEndpointsV1.API_V1_ALL_ROUTES;
+
+/**
+ * Provides the glue code for API definition and the related, i.e. Vertx, 
handlers, etc.
+ * <p>Note that feature-specific routes are defined in the corresponding 
modules, e.g. {@link HealthCheckModule}
+ */
+public class ApiModule extends AbstractModule
+{
+    @Provides
+    @Singleton
+    Router vertxRouter(Vertx vertx, MultiBindingTypeResolver<VertxRoute> 
resolver)
+    {
+        Router router = Router.router(vertx);
+        resolver.resolve().forEach((routeClassKey, route) -> {
+            try
+            {
+                if (RouteClassKey.class.isAssignableFrom(routeClassKey))
+                {
+                    //noinspection unchecked
+                    Class<? extends RouteClassKey> key = (Class<? extends 
RouteClassKey>) routeClassKey;
+                    SettableVertxRoute settableVertxRoute = 
(SettableVertxRoute) route;
+                    settableVertxRoute.setRouteClassKey(key);
+                }
+                route.mountTo(router);
+            }
+            catch (Throwable cause)
+            {
+                throw new RuntimeException("Failed to mount route: " + 
routeClassKey.getSimpleName(), cause);
+            }
+        });
+        return router;
+    }
+
+    @Provides
+    @Singleton
+    Vertx vertx(SidecarConfiguration sidecarConfiguration, 
MetricRegistryFactory metricRegistryFactory)
+    {
+        VertxMetricsConfiguration metricsConfig = 
sidecarConfiguration.metricsConfiguration().vertxConfiguration();
+        Match serverRouteMatch = new 
Match().setValue(API_V1_ALL_ROUTES).setType(MatchType.REGEX);
+        DropwizardMetricsOptions dropwizardMetricsOptions
+        = new DropwizardMetricsOptions().setEnabled(metricsConfig.enabled())
+                                        
.setJmxEnabled(metricsConfig.exposeViaJMX())
+                                        
.setJmxDomain(metricsConfig.jmxDomainName())
+                                        
.setMetricRegistry(metricRegistryFactory.getOrCreate())
+                                        // Monitor all V1 endpoints.
+                                        // Additional filtering is done by 
configuring yaml fields 'metrics.include|exclude'
+                                        
.addMonitoredHttpServerRoute(serverRouteMatch);
+
+        VertxOptions vertxOptions = new 
VertxOptions().setMetricsOptions(dropwizardMetricsOptions);
+        VertxConfiguration vertxConfiguration = 
sidecarConfiguration.vertxConfiguration();
+        FileSystemOptionsConfiguration fsOptions = vertxConfiguration != null 
? vertxConfiguration.filesystemOptionsConfiguration() : null;
+
+        if (fsOptions != null)
+        {
+            vertxOptions.setFileSystemOptions(new FileSystemOptions()
+                                              
.setClassPathResolvingEnabled(fsOptions.classpathResolvingEnabled())
+                                              
.setFileCacheDir(fsOptions.fileCacheDir())
+                                              
.setFileCachingEnabled(fsOptions.fileCachingEnabled()));
+        }
+
+        return Vertx.vertx(vertxOptions);
+    }
+
+    @Provides
+    @Singleton
+    ErrorHandler errorHandler()
+    {
+        return new JsonErrorHandler();
+    }
+
+    @Provides
+    @Singleton
+    LoggerHandler loggerHandler()
+    {
+        return SidecarLoggerHandler.create(LoggerHandler.create());
+    }
+
+    @ProvidesIntoMap
+    @KeyClassMapKey(VertxRouteMapKeys.GlobalUtilityHandlerKey.class)
+    VertxRoute globalUtilityHandler(SidecarConfiguration sidecarConfiguration,
+                                    LoggerHandler loggerHandler)
+    {
+        return VertxRoute.create(router -> {

Review Comment:
   Curious, why are we not building this route with `RouterBuilder.Factory` ?



##########
server/src/main/java/org/apache/cassandra/sidecar/modules/ApiModule.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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.modules;
+
+import com.google.inject.AbstractModule;
+import com.google.inject.Provides;
+import com.google.inject.Singleton;
+import com.google.inject.multibindings.ProvidesIntoMap;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.vertx.core.Vertx;
+import io.vertx.core.VertxOptions;
+import io.vertx.core.file.FileSystemOptions;
+import io.vertx.ext.dropwizard.DropwizardMetricsOptions;
+import io.vertx.ext.dropwizard.Match;
+import io.vertx.ext.dropwizard.MatchType;
+import io.vertx.ext.web.Router;
+import io.vertx.ext.web.handler.ErrorHandler;
+import io.vertx.ext.web.handler.LoggerHandler;
+import io.vertx.ext.web.handler.TimeoutHandler;
+import org.apache.cassandra.sidecar.common.ApiEndpointsV1;
+import org.apache.cassandra.sidecar.config.FileSystemOptionsConfiguration;
+import org.apache.cassandra.sidecar.config.SidecarConfiguration;
+import org.apache.cassandra.sidecar.config.VertxConfiguration;
+import org.apache.cassandra.sidecar.config.VertxMetricsConfiguration;
+import org.apache.cassandra.sidecar.handlers.JsonErrorHandler;
+import org.apache.cassandra.sidecar.handlers.RouteBuilder;
+import org.apache.cassandra.sidecar.handlers.RoutingOrder;
+import org.apache.cassandra.sidecar.handlers.TimeSkewHandler;
+import org.apache.cassandra.sidecar.logging.SidecarLoggerHandler;
+import org.apache.cassandra.sidecar.metrics.MetricRegistryFactory;
+import org.apache.cassandra.sidecar.modules.multibindings.KeyClassMapKey;
+import 
org.apache.cassandra.sidecar.modules.multibindings.MultiBindingTypeResolver;
+import org.apache.cassandra.sidecar.modules.multibindings.RouteClassKey;
+import org.apache.cassandra.sidecar.modules.multibindings.VertxRouteMapKeys;
+import org.apache.cassandra.sidecar.routes.SettableVertxRoute;
+import org.apache.cassandra.sidecar.routes.VertxRoute;
+
+import static 
org.apache.cassandra.sidecar.common.ApiEndpointsV1.API_V1_ALL_ROUTES;
+
+/**
+ * Provides the glue code for API definition and the related, i.e. Vertx, 
handlers, etc.
+ * <p>Note that feature-specific routes are defined in the corresponding 
modules, e.g. {@link HealthCheckModule}
+ */
+public class ApiModule extends AbstractModule
+{
+    @Provides
+    @Singleton
+    Router vertxRouter(Vertx vertx, MultiBindingTypeResolver<VertxRoute> 
resolver)
+    {
+        Router router = Router.router(vertx);
+        resolver.resolve().forEach((routeClassKey, route) -> {
+            try
+            {
+                if (RouteClassKey.class.isAssignableFrom(routeClassKey))
+                {
+                    //noinspection unchecked
+                    Class<? extends RouteClassKey> key = (Class<? extends 
RouteClassKey>) routeClassKey;
+                    SettableVertxRoute settableVertxRoute = 
(SettableVertxRoute) route;
+                    settableVertxRoute.setRouteClassKey(key);
+                }
+                route.mountTo(router);
+            }
+            catch (Throwable cause)
+            {
+                throw new RuntimeException("Failed to mount route: " + 
routeClassKey.getSimpleName(), cause);
+            }
+        });
+        return router;
+    }
+
+    @Provides
+    @Singleton
+    Vertx vertx(SidecarConfiguration sidecarConfiguration, 
MetricRegistryFactory metricRegistryFactory)
+    {
+        VertxMetricsConfiguration metricsConfig = 
sidecarConfiguration.metricsConfiguration().vertxConfiguration();
+        Match serverRouteMatch = new 
Match().setValue(API_V1_ALL_ROUTES).setType(MatchType.REGEX);
+        DropwizardMetricsOptions dropwizardMetricsOptions
+        = new DropwizardMetricsOptions().setEnabled(metricsConfig.enabled())
+                                        
.setJmxEnabled(metricsConfig.exposeViaJMX())
+                                        
.setJmxDomain(metricsConfig.jmxDomainName())
+                                        
.setMetricRegistry(metricRegistryFactory.getOrCreate())
+                                        // Monitor all V1 endpoints.
+                                        // Additional filtering is done by 
configuring yaml fields 'metrics.include|exclude'
+                                        
.addMonitoredHttpServerRoute(serverRouteMatch);
+
+        VertxOptions vertxOptions = new 
VertxOptions().setMetricsOptions(dropwizardMetricsOptions);
+        VertxConfiguration vertxConfiguration = 
sidecarConfiguration.vertxConfiguration();
+        FileSystemOptionsConfiguration fsOptions = vertxConfiguration != null 
? vertxConfiguration.filesystemOptionsConfiguration() : null;
+
+        if (fsOptions != null)
+        {
+            vertxOptions.setFileSystemOptions(new FileSystemOptions()
+                                              
.setClassPathResolvingEnabled(fsOptions.classpathResolvingEnabled())
+                                              
.setFileCacheDir(fsOptions.fileCacheDir())
+                                              
.setFileCachingEnabled(fsOptions.fileCachingEnabled()));
+        }
+
+        return Vertx.vertx(vertxOptions);
+    }
+
+    @Provides
+    @Singleton
+    ErrorHandler errorHandler()
+    {
+        return new JsonErrorHandler();
+    }
+
+    @Provides
+    @Singleton
+    LoggerHandler loggerHandler()
+    {
+        return SidecarLoggerHandler.create(LoggerHandler.create());
+    }
+
+    @ProvidesIntoMap
+    @KeyClassMapKey(VertxRouteMapKeys.GlobalUtilityHandlerKey.class)
+    VertxRoute globalUtilityHandler(SidecarConfiguration sidecarConfiguration,
+                                    LoggerHandler loggerHandler)
+    {
+        return VertxRoute.create(router -> {
+            router.route()
+                  .order(RoutingOrder.HIGHEST.order)
+                  .handler(loggerHandler)
+                  
.handler(TimeoutHandler.create(sidecarConfiguration.serviceConfiguration().requestTimeout().toMillis(),
+                                                 
HttpResponseStatus.REQUEST_TIMEOUT.code()));
+        });
+    }
+
+    @ProvidesIntoMap
+    @KeyClassMapKey(VertxRouteMapKeys.GlobalErrorHandlerKey.class)
+    VertxRoute globalErrorHandler(ErrorHandler errorHandler)
+    {
+        return VertxRoute.create(router -> {
+            router.route()
+                  .path(ApiEndpointsV1.API + "/*")
+                  .order(RoutingOrder.HIGHEST.order)

Review Comment:
   Currently global error handler is lower in priority than `ChainAuthHandler`, 
Having `ApiModule` before `AuthModule` changes this. Also should we rearrange 
this to have `AuthModule` before `ApiModule`? (we can have one more 
`VertxSetupModule` for vertx and router setup and then `AuthModule` and then 
`ApiModule`?)



##########
server/src/main/java/org/apache/cassandra/sidecar/modules/SidecarModules.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * 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.modules;
+
+import java.nio.file.Path;
+import java.util.List;
+
+import com.google.inject.Module;
+import com.google.inject.util.Modules;
+import 
org.apache.cassandra.sidecar.modules.multibindings.MultiBindingTypeResolverModule;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Collection of sidecar modules
+ */
+public class SidecarModules
+{
+    private SidecarModules()
+    {
+        throw new UnsupportedOperationException();
+    }
+
+    /**
+     * All sidecar modules
+     * @param confPath path to the configuration file
+     * @return all sidecar modules
+     */
+    public static List<Module> all(@Nullable Path confPath)
+    {
+        // To prevent unexpected circular dependency chains in your code, we 
recommend that you disable Guice's circular proxy feature.
+        return List.of(Modules.disableCircularProxiesModule(),
+                       new ApiModule(),
+                       new AuthModule(),
+                       new CassandraOperationsModule(),
+                       new CdcModule(),
+                       new ConfigurationModule(confPath),

Review Comment:
   Nit: Is we have `VertxSetupModule` we can move `ConfigurationModule` after 
it for easier reading



##########
server/src/main/java/org/apache/cassandra/sidecar/modules/multibindings/KeyClassMapKey.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.cassandra.sidecar.modules.multibindings;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+import com.google.inject.multibindings.MapKey;
+import com.google.inject.multibindings.ProvidesIntoMap;
+
+/**
+ * Allows {@literal @}{@link ProvidesIntoMap} to specify a {@link ClassKey} 
class as map key.
+ * The class must extends from {@link ClassKey}
+ * <p>The annotation name {@link KeyClassMapKey} is read as "map key type is 
of {@link ClassKey} class"

Review Comment:
   Nit: Shall we name this class to `ClassKeyMapKey` ? that way name matches 
this comment



##########
server/src/main/java/org/apache/cassandra/sidecar/modules/RestoreJobModule.java:
##########
@@ -0,0 +1,195 @@
+/*
+ * 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.modules;
+
+import com.google.inject.AbstractModule;
+import com.google.inject.Provides;
+import com.google.inject.Singleton;
+import com.google.inject.TypeLiteral;
+import com.google.inject.multibindings.MapBinder;
+import com.google.inject.multibindings.ProvidesIntoMap;
+import org.apache.cassandra.sidecar.cluster.InstancesMetadata;
+import org.apache.cassandra.sidecar.cluster.locator.CachedLocalTokenRanges;
+import org.apache.cassandra.sidecar.cluster.locator.LocalTokenRangesProvider;
+import org.apache.cassandra.sidecar.common.server.dns.DnsResolver;
+import org.apache.cassandra.sidecar.config.SidecarConfiguration;
+import org.apache.cassandra.sidecar.db.schema.RestoreJobsSchema;
+import org.apache.cassandra.sidecar.db.schema.RestoreRangesSchema;
+import org.apache.cassandra.sidecar.db.schema.RestoreSlicesSchema;
+import org.apache.cassandra.sidecar.db.schema.TableSchema;
+import org.apache.cassandra.sidecar.handlers.DiskSpaceProtectionHandler;
+import org.apache.cassandra.sidecar.handlers.RouteBuilder;
+import org.apache.cassandra.sidecar.handlers.restore.AbortRestoreJobHandler;
+import org.apache.cassandra.sidecar.handlers.restore.CreateRestoreJobHandler;
+import org.apache.cassandra.sidecar.handlers.restore.CreateRestoreSliceHandler;
+import org.apache.cassandra.sidecar.handlers.restore.RestoreJobProgressHandler;
+import org.apache.cassandra.sidecar.handlers.restore.RestoreJobSummaryHandler;
+import 
org.apache.cassandra.sidecar.handlers.restore.RestoreRequestValidationHandler;
+import org.apache.cassandra.sidecar.handlers.restore.UpdateRestoreJobHandler;
+import 
org.apache.cassandra.sidecar.handlers.validations.ValidateTableExistenceHandler;
+import org.apache.cassandra.sidecar.modules.multibindings.ClassKey;
+import org.apache.cassandra.sidecar.modules.multibindings.KeyClassMapKey;
+import org.apache.cassandra.sidecar.modules.multibindings.PeriodicTaskMapKeys;
+import org.apache.cassandra.sidecar.modules.multibindings.TableSchemaMapKeys;
+import org.apache.cassandra.sidecar.modules.multibindings.VertxRouteMapKeys;
+import org.apache.cassandra.sidecar.restore.RestoreJobDiscoverer;
+import org.apache.cassandra.sidecar.restore.RestoreProcessor;
+import org.apache.cassandra.sidecar.restore.RingTopologyRefresher;
+import org.apache.cassandra.sidecar.routes.VertxRoute;
+import org.apache.cassandra.sidecar.tasks.PeriodicTask;
+
+/**
+ * Provides the capability of restoring data (SSTables) from S3 into Cassandra
+ */
+public class RestoreJobModule extends AbstractModule
+{
+    @Override
+    protected void configure()
+    {
+        // Leverage TypeLiteral to preserve the lower bound type Key at the 
runtime
+        TypeLiteral<Class<? extends ClassKey>> keyType = new TypeLiteral<>() 
{};
+        TypeLiteral<PeriodicTask> valueType = new TypeLiteral<>() {};
+        MapBinder<Class<? extends ClassKey>, PeriodicTask> 
periodicTaskMapBinder = MapBinder.newMapBinder(binder(), keyType, valueType);
+        
periodicTaskMapBinder.addBinding(PeriodicTaskMapKeys.RestoreJobDiscovererKey.class).to(RestoreJobDiscoverer.class);

Review Comment:
   Curious, why do we inject for these periodic tasks and bind them this way. 
Where as for `HealthCheckPeriodicTask` we provide with `ProvidesIntoMap` 



-- 
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