Skip to content

Commit

Permalink
fix: make constants private (#372)
Browse files Browse the repository at this point in the history
  • Loading branch information
supercaracal authored Aug 31, 2024
1 parent ebf75ac commit 981c020
Show file tree
Hide file tree
Showing 16 changed files with 54 additions and 13 deletions.
2 changes: 2 additions & 0 deletions lib/redis_client/cluster.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions lib/redis_client/cluster/command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions lib/redis_client/cluster/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(',')
Expand Down
4 changes: 3 additions & 1 deletion lib/redis_client/cluster/key_slot_converter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
class RedisClient
class Cluster
module KeySlotConverter
HASH_SLOTS = 16_384
EMPTY_STRING = ''
LEFT_BRACKET = '{'
RIGHT_BRACKET = '}'
Expand Down Expand Up @@ -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

Expand Down
5 changes: 5 additions & 0 deletions lib/redis_client/cluster/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions lib/redis_client/cluster/node/base_topology.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions lib/redis_client/cluster/node/latency_replica.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions lib/redis_client/cluster/node_key.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ class Cluster
module NodeKey
DELIMITER = ':'

private_constant :DELIMITER

module_function

def hashify(node_key)
Expand Down
2 changes: 2 additions & 0 deletions lib/redis_client/cluster/normalized_cmd_name.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ class NormalizedCmdName

EMPTY_STRING = ''

private_constant :EMPTY_STRING

def initialize
@cache = {}
@mutex = Mutex.new
Expand Down
2 changes: 2 additions & 0 deletions lib/redis_client/cluster/pub_sub.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions lib/redis_client/cluster/router.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions lib/redis_client/cluster/transaction.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions lib/redis_client/cluster_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions test/cluster_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion test/ips_slot_node_mapping.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

module IpsSlotNodeMapping
ELEMENTS = %w[foo bar baz].freeze
SIZE = ::RedisClient::Cluster::Node::SLOT_SIZE
SIZE = 16_384

module_function

Expand Down
26 changes: 15 additions & 11 deletions test/redis_client/cluster/test_node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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!)
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 981c020

Please sign in to comment.