review fixes

tysion · Apr 20, 2024 · ca60361 · ca60361
1 parent e8f62d2
commit ca60361
Show file tree

Hide file tree

Showing 12 changed files with 27 additions and 31 deletions.
diff --git a/redis/include/userver/storages/redis/impl/base.hpp b/redis/include/userver/storages/redis/impl/base.hpp
@@ -27,13 +27,13 @@ struct ConnectionInfo {
   bool read_only = false;
   ConnectionSecurity connection_security = ConnectionSecurity::kNone;
   using HostVector = std::vector<std::string>;
-  std::optional<size_t> database_index{};
+  size_t database_index = 0;
 
   ConnectionInfo() = default;
   ConnectionInfo(std::string host, int port, Password password,
                  bool read_only = false,
                  ConnectionSecurity security = ConnectionSecurity::kNone,
-                 std::optional<size_t> database_index = {})
+                 size_t database_index = {})
       : host{std::move(host)},
         port{port},
         password{std::move(password)},

diff --git a/redis/include/userver/storages/redis/impl/secdist_redis.hpp b/redis/include/userver/storages/redis/impl/secdist_redis.hpp
@@ -21,7 +21,7 @@ struct RedisSettings {
 
   std::vector<std::string> shards;
   std::vector<HostPort> sentinels;
-  std::optional<size_t> database_index;
+  size_t database_index{0};
   redis::Password password{std::string()};
   redis::ConnectionSecurity secure_connection{redis::ConnectionSecurity::kNone};
 };

diff --git a/redis/src/storages/redis/impl/redis.cpp b/redis/src/storages/redis/impl/redis.cpp
@@ -147,7 +147,7 @@ class Redis::RedisImpl : public std::enable_shared_from_this<Redis::RedisImpl> {
   ~RedisImpl();
 
   void Connect(const ConnectionInfo::HostVector& host_addrs, int port,
-               const Password& password, std::optional<size_t> database_index);
+               const Password& password, size_t database_index);
   void Disconnect();
 
   bool AsyncCommand(const CommandPtr& command);
@@ -254,7 +254,7 @@ class Redis::RedisImpl : public std::enable_shared_from_this<Redis::RedisImpl> {
       const CommandsBufferingSettings& commands_buffering_settings);
 
   bool Connect(const std::string& host, int port, const Password& password,
-               std::optional<size_t> database_index);
+               size_t database_index);
 
   Redis* redis_obj_;
   engine::ev::ThreadControl ev_thread_control_;
@@ -275,7 +275,7 @@ class Redis::RedisImpl : public std::enable_shared_from_this<Redis::RedisImpl> {
   uint16_t port_ = 0;
   std::string server_;
   Password password_{std::string()};
-  std::optional<size_t> database_index_;
+  std::size_t database_index_ = 0;
   std::atomic<size_t> commands_size_ = 0;
   size_t sent_count_ = 0;
   size_t cmd_counter_ = 0;
@@ -341,7 +341,7 @@ Redis::~Redis() {
 
 void Redis::Connect(const ConnectionInfo::HostVector& host_addrs, int port,
                     const Password& password,
-                    std::optional<size_t> database_index) {
+                    size_t database_index) {
   impl_->Connect(host_addrs, port, password, database_index);
 }
 
@@ -451,7 +451,7 @@ void Redis::RedisImpl::Detach() {
 
 void Redis::RedisImpl::Connect(const ConnectionInfo::HostVector& host_addrs,
                                int port, const Password& password,
-                               std::optional<size_t> database_index) {
+                               size_t database_index) {
   for (const auto& host : host_addrs)
     if (Connect(host, port, password, database_index)) return;
 
@@ -462,7 +462,7 @@ void Redis::RedisImpl::Connect(const ConnectionInfo::HostVector& host_addrs,
 
 bool Redis::RedisImpl::Connect(const std::string& host, int port,
                                const Password& password,
-                               std::optional<size_t> database_index) {
+                               size_t database_index) {
   UASSERT(context_ == nullptr);
   UASSERT(state_ == State::kInit);
 
@@ -1109,19 +1109,21 @@ void Redis::RedisImpl::SendReadOnly() {
 }
 
 void Redis::RedisImpl::SelectDatabase() {
-  if (!database_index_) {
+  // To get rid of the redundant `SELECT 0` command
+  // since 0 is the default database index, and it will be set automatically
+  if (database_index_ == 0) {
     SetState(RedisState::kConnected);
     return;
   }
 
   ProcessCommand(PrepareCommand(
-      CmdArgs{"SELECT", *database_index_},
+      CmdArgs{"SELECT", database_index_},
       [this](const CommandPtr&, ReplyPtr reply) {
         if (*reply && reply->data.IsStatus()) {
           SetState(RedisState::kConnected);
           LOG_INFO() << log_extra_
                      << "Selected redis logical database with index "
-                     << *database_index_;
+                     << database_index_;
           return;
         }
 

diff --git a/redis/src/storages/redis/impl/redis.hpp b/redis/src/storages/redis/impl/redis.hpp
@@ -34,7 +34,7 @@ class Redis {
 
   void Connect(const ConnectionInfo::HostVector& host_addrs, int port,
                const Password& password,
-               std::optional<size_t> database_index = {});
+               size_t database_index = 0);
 
   bool AsyncCommand(const CommandPtr& command);
   size_t GetRunningCommands() const;

diff --git a/redis/src/storages/redis/impl/sentinel.cpp b/redis/src/storages/redis/impl/sentinel.cpp
@@ -74,7 +74,7 @@ Sentinel::Sentinel(const std::shared_ptr<ThreadPools>& thread_pools,
                    std::unique_ptr<KeyShard>&& key_shard,
                    CommandControl command_control,
                    const testsuite::RedisControl& testsuite_redis_control,
-                   ConnectionMode mode, std::optional<size_t> database_index)
+                   ConnectionMode mode, size_t database_index)
     : thread_pools_(thread_pools),
       secdist_default_command_control_(command_control),
       testsuite_redis_control_(testsuite_redis_control) {
@@ -169,7 +169,7 @@ std::shared_ptr<Sentinel> Sentinel::CreateSentinel(
     // sentinels in cluster mode.
     conns.emplace_back(sentinel.host, sentinel.port,
                        (key_shard ? Password("") : password), false,
-                       settings.secure_connection, std::nullopt);
+                       settings.secure_connection);
   }
 
   LOG_DEBUG() << "redis command_control:" << command_control.ToString();

diff --git a/redis/src/storages/redis/impl/sentinel.hpp b/redis/src/storages/redis/impl/sentinel.hpp
@@ -72,7 +72,7 @@ class Sentinel {
            CommandControl command_control = {},
            const testsuite::RedisControl& testsuite_redis_control = {},
            ConnectionMode mode = ConnectionMode::kCommands,
-           std::optional<size_t> database_index = {});
+           size_t database_index = 0);
   virtual ~Sentinel();
 
   void Start();

diff --git a/redis/src/storages/redis/impl/sentinel_impl.cpp b/redis/src/storages/redis/impl/sentinel_impl.cpp
@@ -65,7 +65,7 @@ SentinelImpl::SentinelImpl(
     ConnectionSecurity connection_security, ReadyChangeCallback ready_callback,
     std::unique_ptr<KeyShard>&& key_shard,
     dynamic_config::Source dynamic_config_source, ConnectionMode mode,
-    std::optional<size_t> database_index)
+    size_t database_index)
     : sentinel_obj_(sentinel),
       ev_thread_(sentinel_thread_control),
       shard_group_name_(std::move(shard_group_name)),

diff --git a/redis/src/storages/redis/impl/sentinel_impl.hpp b/redis/src/storages/redis/impl/sentinel_impl.hpp
@@ -113,7 +113,7 @@ class SentinelImpl : public SentinelImplBase {
                std::unique_ptr<KeyShard>&& key_shard,
                dynamic_config::Source dynamic_config_source,
                ConnectionMode mode = ConnectionMode::kCommands,
-               std::optional<size_t> database_index = {});
+               size_t database_index = 0);
   ~SentinelImpl() override;
 
   std::unordered_map<ServerId, size_t, ServerIdHasher>
@@ -286,7 +286,7 @@ class SentinelImpl : public SentinelImplBase {
   std::optional<CommandsBufferingSettings> commands_buffering_settings_;
   dynamic_config::Source dynamic_config_source_;
   std::atomic<int> publish_shard_{0};
-  std::optional<size_t> database_index_;
+  std::size_t database_index_{0};
 };
 
 }  // namespace redis

diff --git a/redis/src/storages/redis/impl/shard.cpp b/redis/src/storages/redis/impl/shard.cpp
@@ -34,11 +34,11 @@ void ConnectionInfoInt::SetPassword(Password password) {
   conn_info_.password = std::move(password);
 }
 
-void ConnectionInfoInt::SetDatabaseIndex(std::optional<size_t> index) {
+void ConnectionInfoInt::SetDatabaseIndex(size_t index) {
   conn_info_.database_index = index;
 }
 
-std::optional<size_t> ConnectionInfoInt::DatabaseIndex() const {
+size_t ConnectionInfoInt::DatabaseIndex() const {
   return conn_info_.database_index;
 }
 

diff --git a/redis/src/storages/redis/impl/shard.hpp b/redis/src/storages/redis/impl/shard.hpp
@@ -28,8 +28,8 @@ class ConnectionInfoInt {
 
   void SetPassword(Password);
 
-  void SetDatabaseIndex(std::optional<size_t> index);
-  std::optional<size_t> DatabaseIndex() const;
+  void SetDatabaseIndex(size_t index);
+  size_t DatabaseIndex() const;
 
   bool IsReadOnly() const;
   void SetReadOnly(bool);

diff --git a/redis/src/storages/redis/redis_secdist.cpp b/redis/src/storages/redis/redis_secdist.cpp
@@ -1,6 +1,5 @@
 #include "redis_secdist.hpp"
 
-#include <userver/formats/parse/common_containers.hpp>
 #include <userver/logging/log.hpp>
 #include <userver/storages/secdist/exceptions.hpp>
 #include <userver/storages/secdist/helpers.hpp>
@@ -44,13 +43,7 @@ RedisMapSettings::RedisMapSettings(const formats::json::Value& doc) {
             ? USERVER_NAMESPACE::redis::ConnectionSecurity::kTLS
             : USERVER_NAMESPACE::redis::ConnectionSecurity::kNone;
 
-    settings.database_index =
-        client_settings["database_index"].As<std::optional<size_t>>();
-    if (settings.database_index && *settings.database_index == 0) {
-      // To get rid of the redundant `SELECT 0` command
-      // since 0 is the default database index and it will be set automatically
-      settings.database_index = std::nullopt;
-    }
+    settings.database_index = client_settings["database_index"].As<size_t>(0);
 
     const auto& shards = client_settings["shards"];
     CheckIsArray(shards, "shards");

diff --git a/samples/redis_service/tests/conftest.py b/samples/redis_service/tests/conftest.py
@@ -14,6 +14,7 @@ def service_env(redis_sentinels):
         'redis_settings': {
             'taxi-tmp': {
                 'password': '',
+                'database_index': 0,
                 'sentinels': redis_sentinels,
                 'shards': [{'name': 'test_master0'}],
             },