From 981c020b2752406c0ad9f73e823e61490298affc Mon Sep 17 00:00:00 2001 From: Taishi Kasuga Date: Sat, 31 Aug 2024 12:08:38 +0900 Subject: [PATCH] fix: make constants private (#372) --- lib/redis_client/cluster.rb | 2 ++ lib/redis_client/cluster/command.rb | 2 ++ lib/redis_client/cluster/errors.rb | 2 ++ .../cluster/key_slot_converter.rb | 4 ++- lib/redis_client/cluster/node.rb | 5 ++++ .../cluster/node/base_topology.rb | 2 ++ .../cluster/node/latency_replica.rb | 2 ++ lib/redis_client/cluster/node_key.rb | 2 ++ .../cluster/normalized_cmd_name.rb | 2 ++ lib/redis_client/cluster/pub_sub.rb | 2 ++ lib/redis_client/cluster/router.rb | 4 +++ lib/redis_client/cluster/transaction.rb | 3 +++ lib/redis_client/cluster_config.rb | 4 +++ test/cluster_controller.rb | 3 +++ test/ips_slot_node_mapping.rb | 2 +- test/redis_client/cluster/test_node.rb | 26 +++++++++++-------- 16 files changed, 54 insertions(+), 13 deletions(-) diff --git a/lib/redis_client/cluster.rb b/lib/redis_client/cluster.rb index 9d945521..ceacde1f 100644 --- a/lib/redis_client/cluster.rb +++ b/lib/redis_client/cluster.rb @@ -11,6 +11,8 @@ class RedisClient class Cluster ZERO_CURSOR_FOR_SCAN = '0' + private_constant :ZERO_CURSOR_FOR_SCAN + attr_reader :config def initialize(config, pool: nil, concurrency: nil, **kwargs) diff --git a/lib/redis_client/cluster/command.rb b/lib/redis_client/cluster/command.rb index 5c82df1a..0a6f390d 100644 --- a/lib/redis_client/cluster/command.rb +++ b/lib/redis_client/cluster/command.rb @@ -12,6 +12,8 @@ class Command EMPTY_HASH = {}.freeze EMPTY_ARRAY = [].freeze + private_constant :EMPTY_STRING, :EMPTY_HASH, :EMPTY_ARRAY + Detail = Struct.new( 'RedisCommand', :first_key_position, diff --git a/lib/redis_client/cluster/errors.rb b/lib/redis_client/cluster/errors.rb index 0367b3d4..77a66d71 100644 --- a/lib/redis_client/cluster/errors.rb +++ b/lib/redis_client/cluster/errors.rb @@ -6,6 +6,8 @@ class RedisClient class Cluster ERR_ARG_NORMALIZATION = ->(arg) { Array[arg].flatten.reject { |e| e.nil? || (e.respond_to?(:empty?) && e.empty?) } } + private_constant :ERR_ARG_NORMALIZATION + class InitialSetupError < ::RedisClient::Error def initialize(errors) msg = ERR_ARG_NORMALIZATION.call(errors).map(&:message).uniq.join(',') diff --git a/lib/redis_client/cluster/key_slot_converter.rb b/lib/redis_client/cluster/key_slot_converter.rb index 37012717..0f05dd64 100644 --- a/lib/redis_client/cluster/key_slot_converter.rb +++ b/lib/redis_client/cluster/key_slot_converter.rb @@ -3,6 +3,7 @@ class RedisClient class Cluster module KeySlotConverter + HASH_SLOTS = 16_384 EMPTY_STRING = '' LEFT_BRACKET = '{' RIGHT_BRACKET = '}' @@ -41,7 +42,8 @@ module KeySlotConverter 0x6e17, 0x7e36, 0x4e55, 0x5e74, 0x2e93, 0x3eb2, 0x0ed1, 0x1ef0 ].freeze - HASH_SLOTS = 16_384 + private_constant :HASH_SLOTS, :EMPTY_STRING, + :LEFT_BRACKET, :RIGHT_BRACKET, :XMODEM_CRC16_LOOKUP module_function diff --git a/lib/redis_client/cluster/node.rb b/lib/redis_client/cluster/node.rb index 0b0e7ed5..e85cb7d2 100644 --- a/lib/redis_client/cluster/node.rb +++ b/lib/redis_client/cluster/node.rb @@ -24,6 +24,9 @@ class Node EMPTY_ARRAY = [].freeze EMPTY_HASH = {}.freeze + private_constant :USE_CHAR_ARRAY_SLOT, :SLOT_SIZE, :MIN_SLOT, :MAX_SLOT, + :DEAD_FLAGS, :ROLE_FLAGS, :EMPTY_ARRAY, :EMPTY_HASH + ReloadNeeded = Class.new(::RedisClient::Error) Info = Struct.new( @@ -45,6 +48,8 @@ class CharArray BASE = '' PADDING = '0' + private_constant :BASE, :PADDING + def initialize(size, elements) @elements = elements @string = String.new(BASE, encoding: Encoding::BINARY, capacity: size) diff --git a/lib/redis_client/cluster/node/base_topology.rb b/lib/redis_client/cluster/node/base_topology.rb index d24c91a4..82c5f2ca 100644 --- a/lib/redis_client/cluster/node/base_topology.rb +++ b/lib/redis_client/cluster/node/base_topology.rb @@ -8,6 +8,8 @@ class BaseTopology EMPTY_HASH = {}.freeze EMPTY_ARRAY = [].freeze + private_constant :IGNORE_GENERIC_CONFIG_KEYS, :EMPTY_HASH, :EMPTY_ARRAY + attr_reader :clients, :primary_clients, :replica_clients def initialize(pool, concurrent_worker, **kwargs) diff --git a/lib/redis_client/cluster/node/latency_replica.rb b/lib/redis_client/cluster/node/latency_replica.rb index b1636947..9d5eaf7f 100644 --- a/lib/redis_client/cluster/node/latency_replica.rb +++ b/lib/redis_client/cluster/node/latency_replica.rb @@ -9,6 +9,8 @@ class LatencyReplica < BaseTopology DUMMY_LATENCY_MSEC = 100 * 1000 * 1000 MEASURE_ATTEMPT_COUNT = 10 + private_constant :DUMMY_LATENCY_MSEC, :MEASURE_ATTEMPT_COUNT + def clients_for_scanning(seed: nil) # rubocop:disable Lint/UnusedMethodArgument @clients_for_scanning end diff --git a/lib/redis_client/cluster/node_key.rb b/lib/redis_client/cluster/node_key.rb index c62ad80f..c48b7c71 100644 --- a/lib/redis_client/cluster/node_key.rb +++ b/lib/redis_client/cluster/node_key.rb @@ -8,6 +8,8 @@ class Cluster module NodeKey DELIMITER = ':' + private_constant :DELIMITER + module_function def hashify(node_key) diff --git a/lib/redis_client/cluster/normalized_cmd_name.rb b/lib/redis_client/cluster/normalized_cmd_name.rb index c0232f29..c1f1f78f 100644 --- a/lib/redis_client/cluster/normalized_cmd_name.rb +++ b/lib/redis_client/cluster/normalized_cmd_name.rb @@ -9,6 +9,8 @@ class NormalizedCmdName EMPTY_STRING = '' + private_constant :EMPTY_STRING + def initialize @cache = {} @mutex = Mutex.new diff --git a/lib/redis_client/cluster/pub_sub.rb b/lib/redis_client/cluster/pub_sub.rb index 36b1a3e8..3b1ba45d 100644 --- a/lib/redis_client/cluster/pub_sub.rb +++ b/lib/redis_client/cluster/pub_sub.rb @@ -44,6 +44,8 @@ def spawn_worker(client, queue) BUF_SIZE = Integer(ENV.fetch('REDIS_CLIENT_PUBSUB_BUF_SIZE', 1024)) + private_constant :BUF_SIZE + def initialize(router, command_builder) @router = router @command_builder = command_builder diff --git a/lib/redis_client/cluster/router.rb b/lib/redis_client/cluster/router.rb index a7cf599f..7bd9fdc0 100644 --- a/lib/redis_client/cluster/router.rb +++ b/lib/redis_client/cluster/router.rb @@ -19,6 +19,8 @@ class Router ZERO_CURSOR_FOR_SCAN = '0' TSF = ->(f, x) { f.nil? ? x : f.call(x) }.curry + private_constant :ZERO_CURSOR_FOR_SCAN, :TSF + def initialize(config, concurrent_worker, pool: nil, **kwargs) @config = config.dup @original_config = config.dup if config.connect_with_original_config @@ -335,6 +337,8 @@ def send_watch_command(command) 'del' => ['del', 1].freeze }.freeze + private_constant :MULTIPLE_KEYS_COMMAND_TO_SINGLE + def send_multiple_keys_command(cmd, method, command, args, &block) # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity # This implementation is prioritized performance rather than readability or so. single_key_cmd, keys_step = MULTIPLE_KEYS_COMMAND_TO_SINGLE.fetch(cmd) diff --git a/lib/redis_client/cluster/transaction.rb b/lib/redis_client/cluster/transaction.rb index 7db93ef7..a7f74136 100644 --- a/lib/redis_client/cluster/transaction.rb +++ b/lib/redis_client/cluster/transaction.rb @@ -7,9 +7,12 @@ class RedisClient class Cluster class Transaction ConsistencyError = Class.new(::RedisClient::Error) + MAX_REDIRECTION = 2 EMPTY_ARRAY = [].freeze + private_constant :MAX_REDIRECTION, :EMPTY_ARRAY + def initialize(router, command_builder, node: nil, slot: nil, asking: false) @router = router @command_builder = command_builder diff --git a/lib/redis_client/cluster_config.rb b/lib/redis_client/cluster_config.rb index 75aaf95d..9fbd76d5 100644 --- a/lib/redis_client/cluster_config.rb +++ b/lib/redis_client/cluster_config.rb @@ -23,6 +23,10 @@ class ClusterConfig # It affects to strike a balance between load and stability in initialization or changed states. MAX_STARTUP_SAMPLE = Integer(ENV.fetch('REDIS_CLIENT_MAX_STARTUP_SAMPLE', 3)) + private_constant :DEFAULT_HOST, :DEFAULT_PORT, :DEFAULT_SCHEME, :SECURE_SCHEME, :DEFAULT_NODES, + :VALID_SCHEMES, :VALID_NODES_KEYS, :MERGE_CONFIG_KEYS, :IGNORE_GENERIC_CONFIG_KEYS, + :MAX_WORKERS, :SLOW_COMMAND_TIMEOUT, :MAX_STARTUP_SAMPLE + InvalidClientConfigError = Class.new(::RedisClient::Error) attr_reader :command_builder, :client_config, :replica_affinity, :slow_command_timeout, diff --git a/test/cluster_controller.rb b/test/cluster_controller.rb index a37bf912..ebc87779 100644 --- a/test/cluster_controller.rb +++ b/test/cluster_controller.rb @@ -9,6 +9,9 @@ class ClusterController DEFAULT_MAX_ATTEMPTS = 600 DEFAULT_TIMEOUT_SEC = 5.0 + private_constant :SLOT_SIZE, :DEFAULT_SHARD_SIZE, :DEFAULT_REPLICA_SIZE, + :DEFAULT_MAX_ATTEMPTS, :DEFAULT_TIMEOUT_SEC + RedisNodeInfo = Struct.new( 'RedisClusterNodeInfo', :id, :node_key, :flags, :role, :myself?, :primary_id, :ping_sent, :pong_recv, diff --git a/test/ips_slot_node_mapping.rb b/test/ips_slot_node_mapping.rb index 04b6176e..5ecacb06 100644 --- a/test/ips_slot_node_mapping.rb +++ b/test/ips_slot_node_mapping.rb @@ -5,7 +5,7 @@ module IpsSlotNodeMapping ELEMENTS = %w[foo bar baz].freeze - SIZE = ::RedisClient::Cluster::Node::SLOT_SIZE + SIZE = 16_384 module_function diff --git a/test/redis_client/cluster/test_node.rb b/test/redis_client/cluster/test_node.rb index 11ab7152..1f1c42d7 100644 --- a/test/redis_client/cluster/test_node.rb +++ b/test/redis_client/cluster/test_node.rb @@ -22,6 +22,10 @@ def test_connection_prelude # rubocop:disable Metrics/ClassLength class TestNode < TestingWrapper + USE_CHAR_ARRAY_SLOT = Integer(ENV.fetch('REDIS_CLIENT_USE_CHAR_ARRAY_SLOT', 1)) == 1 + SLOT_SIZE = 16_384 + MAX_STARTUP_SAMPLE = Integer(ENV.fetch('REDIS_CLIENT_MAX_STARTUP_SAMPLE', 3)) + def setup @test_node = make_node.tap(&:reload!) @test_node_with_scale_read = make_node(replica: true).tap(&:reload!) @@ -467,8 +471,8 @@ def test_make_array_for_slot_node_mappings_optimized want = node_info_list.first.node_key got = @test_node.send(:make_array_for_slot_node_mappings, node_info_list) - assert_instance_of(::RedisClient::Cluster::Node::USE_CHAR_ARRAY_SLOT ? ::RedisClient::Cluster::Node::CharArray : Array, got) - ::RedisClient::Cluster::Node::SLOT_SIZE.times do |i| + assert_instance_of(USE_CHAR_ARRAY_SLOT ? ::RedisClient::Cluster::Node::CharArray : Array, got) + SLOT_SIZE.times do |i| got[i] = want assert_equal(want, got[i], "Case: #{i}") end @@ -485,7 +489,7 @@ def test_make_array_for_slot_node_mappings_unoptimized want = node_info_list.first.node_key got = @test_node.send(:make_array_for_slot_node_mappings, node_info_list) assert_instance_of(Array, got) - ::RedisClient::Cluster::Node::SLOT_SIZE.times do |i| + SLOT_SIZE.times do |i| got[i] = want assert_equal(want, got[i], "Case: #{i}") end @@ -500,19 +504,19 @@ def test_make_array_for_slot_node_mappings_max_shard_size end got = @test_node.send(:make_array_for_slot_node_mappings, node_info_list) - assert_instance_of(::RedisClient::Cluster::Node::USE_CHAR_ARRAY_SLOT ? ::RedisClient::Cluster::Node::CharArray : Array, got) + assert_instance_of(USE_CHAR_ARRAY_SLOT ? ::RedisClient::Cluster::Node::CharArray : Array, got) - ::RedisClient::Cluster::Node::SLOT_SIZE.times { |i| got[i] = node_info_list.first.node_key } + SLOT_SIZE.times { |i| got[i] = node_info_list.first.node_key } got[0] = 'newbie:6379' assert_equal('newbie:6379', got[0]) - assert_raises(RangeError) { got[0] = 'zombie:6379' } if ::RedisClient::Cluster::Node::USE_CHAR_ARRAY_SLOT + assert_raises(RangeError) { got[0] = 'zombie:6379' } if USE_CHAR_ARRAY_SLOT - assert_raises(IndexError) { got[-1] = 'newbie:6379' } if ::RedisClient::Cluster::Node::USE_CHAR_ARRAY_SLOT - assert_raises(IndexError) { got[-1] } if ::RedisClient::Cluster::Node::USE_CHAR_ARRAY_SLOT + assert_raises(IndexError) { got[-1] = 'newbie:6379' } if USE_CHAR_ARRAY_SLOT + assert_raises(IndexError) { got[-1] } if USE_CHAR_ARRAY_SLOT got[16_384] = 'newbie:6379' - assert_nil(got[16_384]) if ::RedisClient::Cluster::Node::USE_CHAR_ARRAY_SLOT + assert_nil(got[16_384]) if USE_CHAR_ARRAY_SLOT end def test_build_replication_mappings_regular @@ -600,7 +604,7 @@ def test_reload # It should have reloaded by calling CLUSTER NODES on three of the startup nodes cluster_node_cmds = capture_buffer.to_a.select { |c| c.command == %w[CLUSTER NODES] } - assert_equal RedisClient::ClusterConfig::MAX_STARTUP_SAMPLE, cluster_node_cmds.size + assert_equal MAX_STARTUP_SAMPLE, cluster_node_cmds.size # It should have connected to all of the clients. assert_equal TEST_NUMBER_OF_NODES, test_node.to_a.size @@ -677,7 +681,7 @@ def refetch_node_info_list(...) # We should only have reloaded once, which is to say, we only called CLUSTER NODES command MAX_STARTUP_SAMPLE # times cluster_node_cmds = capture_buffer.to_a.select { |c| c.command == %w[CLUSTER NODES] } - assert_equal RedisClient::ClusterConfig::MAX_STARTUP_SAMPLE, cluster_node_cmds.size + assert_equal MAX_STARTUP_SAMPLE, cluster_node_cmds.size end end # rubocop:enable Metrics/ClassLength