From 55c4fd045eeec87b60f4aa9771592537aa500e5a Mon Sep 17 00:00:00 2001 From: Matt Larraz Date: Fri, 22 Nov 2024 15:26:01 -0500 Subject: [PATCH] Run Rubocop and fix errors --- .github/workflows/CI.yml | 11 +++++ .rubocop.yml | 88 +++++++++++++++++++------------------ Gemfile | 7 +++ Rakefile | 2 + bin/console | 2 + lib/suo.rb | 2 + lib/suo/client/base.rb | 42 +++++++++--------- lib/suo/client/memcached.rb | 2 + lib/suo/client/redis.rb | 10 +++-- lib/suo/errors.rb | 2 + lib/suo/version.rb | 4 +- suo.gemspec | 14 +++--- test/client_test.rb | 44 ++++++++++++++----- test/test_helper.rb | 5 ++- 14 files changed, 143 insertions(+), 92 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 602abb5..1249c29 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -35,3 +35,14 @@ jobs: bundler-cache: true - run: | bundle exec rake + + rubocop: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: ruby/setup-ruby@v1 + with: + ruby-version: ruby + bundler-cache: true + - run: | + bundle exec rubocop diff --git a/.rubocop.yml b/.rubocop.yml index bb17161..ef21a0e 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,8 +1,10 @@ AllCops: + NewCops: enable + TargetRubyVersion: 3.1 Exclude: - .git/**/* - tmp/**/* - - suo.gemspec + - vendor/**/* Lint/DuplicateMethods: Enabled: true @@ -10,13 +12,13 @@ Lint/DuplicateMethods: Lint/DeprecatedClassMethods: Enabled: true -Style/TrailingWhitespace: +Layout/TrailingWhitespace: Enabled: true -Style/Tab: +Layout/IndentationStyle: Enabled: true -Style/TrailingBlankLines: +Layout/TrailingEmptyLines: Enabled: true Style/NilComparison: @@ -34,7 +36,7 @@ Style/RedundantReturn: Style/ClassCheck: Enabled: true -Style/EmptyLines: +Layout/EmptyLines: Enabled: true Style/EmptyLiteral: @@ -43,80 +45,79 @@ Style/EmptyLiteral: Style/Alias: Enabled: true -Style/MethodCallParentheses: +Style/MethodCallWithoutArgsParentheses: Enabled: true Style/MethodDefParentheses: Enabled: true -Style/SpaceBeforeBlockBraces: +Layout/SpaceBeforeBlockBraces: Enabled: true -Style/SpaceInsideBlockBraces: +Layout/SpaceInsideBlockBraces: Enabled: true -Style/SpaceInsideParens: +Layout/SpaceInsideParens: Enabled: true -Style/DeprecatedHashMethods: +Style/PreferredHashMethods: Enabled: true Style/HashSyntax: Enabled: true -Style/SpaceInsideHashLiteralBraces: +Layout/SpaceInsideHashLiteralBraces: Enabled: true EnforcedStyle: no_space -Style/SpaceInsideBrackets: - Enabled: true - Style/AndOr: Enabled: false -Style/TrailingCommaInLiteral: +Style/TrailingCommaInArrayLiteral: Enabled: true + EnforcedStyleForMultiline: consistent_comma -Style/SpaceBeforeComma: +Style/TrailingCommaInHashLiteral: Enabled: true + EnforcedStyleForMultiline: consistent_comma -Style/SpaceBeforeComment: +Layout/SpaceBeforeComma: Enabled: true -Style/SpaceBeforeSemicolon: +Layout/SpaceBeforeComment: Enabled: true -Style/SpaceAroundBlockParameters: +Layout/SpaceBeforeSemicolon: Enabled: true -Style/SpaceAroundOperators: +Layout/SpaceAroundBlockParameters: Enabled: true -Style/SpaceAfterColon: +Layout/SpaceAroundOperators: Enabled: true -Style/SpaceAfterComma: +Layout/SpaceAfterColon: Enabled: true -Style/SpaceAroundKeyword: +Layout/SpaceAfterComma: Enabled: true -Style/SpaceAfterNot: +Layout/SpaceAroundKeyword: Enabled: true -Style/SpaceAfterSemicolon: +Layout/SpaceAfterNot: Enabled: true -Lint/UselessComparison: +Layout/SpaceAfterSemicolon: Enabled: true -Lint/InvalidCharacterLiteral: +Lint/BinaryOperatorWithIdenticalOperands: Enabled: true Lint/LiteralInInterpolation: Enabled: true -Lint/LiteralInCondition: +Lint/LiteralAsCondition: Enabled: true Lint/UnusedBlockArgument: @@ -134,19 +135,19 @@ Style/ParenthesesAroundCondition: Style/WhileUntilDo: Enabled: true -Style/EmptyLineBetweenDefs: +Layout/EmptyLineBetweenDefs: Enabled: true -Style/EmptyLinesAroundAccessModifier: +Layout/EmptyLinesAroundAccessModifier: Enabled: true -Style/EmptyLinesAroundMethodBody: +Layout/EmptyLinesAroundMethodBody: Enabled: true Style/ColonMethodCall: Enabled: true -Lint/SpaceBeforeFirstArg: +Layout/SpaceBeforeFirstArg: Enabled: true Lint/UnreachableCode: @@ -165,7 +166,7 @@ Style/StringLiterals: Metrics/CyclomaticComplexity: Max: 10 -Metrics/LineLength: +Layout/LineLength: Max: 128 Metrics/MethodLength: @@ -174,8 +175,18 @@ Metrics/MethodLength: Metrics/PerceivedComplexity: Max: 8 +Naming/BlockForwarding: + EnforcedStyle: explicit + +Metrics/ModuleLength: + Exclude: + - "test/**/*" + # Disabled +Style/Documentation: + Enabled: false + Style/EvenOdd: Enabled: false @@ -185,7 +196,7 @@ Style/AsciiComments: Style/NumericLiterals: Enabled: false -Style/UnneededPercentQ: +Style/RedundantPercentQ: Enabled: false Style/SpecialGlobalVars: @@ -206,14 +217,5 @@ Metrics/BlockNesting: Metrics/ClassLength: Enabled: false -Metrics/MethodLength: - Enabled: false - Metrics/ParameterLists: Enabled: false - -Metrics/PerceivedComplexity: - Enabled: false - -Style/Documentation: - Enabled: false diff --git a/Gemfile b/Gemfile index b4e2a20..93e905f 100644 --- a/Gemfile +++ b/Gemfile @@ -1,3 +1,10 @@ +# frozen_string_literal: true + source "https://rubygems.org" +gem "codeclimate-test-reporter", "~> 0.4.7" +gem "minitest", "~> 5.25" +gem "rake", "~> 13.0" +gem "rubocop", "~> 1.68.0" + gemspec diff --git a/Rakefile b/Rakefile index 5cda719..37cf67e 100644 --- a/Rakefile +++ b/Rakefile @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "bundler/gem_tasks" require "rake/testtask" diff --git a/bin/console b/bin/console index eba769f..4d3f106 100755 --- a/bin/console +++ b/bin/console @@ -1,5 +1,7 @@ #!/usr/bin/env ruby +# frozen_string_literal: true + require "bundler/setup" require "suo" require "irb" diff --git a/lib/suo.rb b/lib/suo.rb index 077941c..07d1220 100644 --- a/lib/suo.rb +++ b/lib/suo.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "securerandom" require "monitor" diff --git a/lib/suo/client/base.rb b/lib/suo/client/base.rb index 7fc87f7..0b94406 100644 --- a/lib/suo/client/base.rb +++ b/lib/suo/client/base.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Suo module Client class Base @@ -9,14 +11,14 @@ class Base ttl: 60, }.freeze - BLANK_STR = "".freeze + BLANK_STR = "" attr_accessor :client, :key, :resources, :options include MonitorMixin def initialize(key, options = {}) - fail "Client required" unless options[:client] + raise "Client required" unless options[:client] @options = DEFAULT_OPTIONS.merge(options) @retry_count = (@options[:acquisition_timeout] / @options[:acquisition_delay].to_f).ceil @@ -46,10 +48,8 @@ def locked? end def locks - val, _ = get - cleared_locks = deserialize_and_clear_locks(val) - - cleared_locks + val, = get + deserialize_and_clear_locks(val) end def refresh(token) @@ -81,12 +81,12 @@ def unlock(token) break unless acquisition_lock break if set(serialize_locks(cleared_locks), cas, expire: cleared_locks.empty?) end - rescue LockClientError => _ # rubocop:disable Lint/HandleExceptions + rescue LockClientError => _e # ignore - assume success due to optimistic locking end def clear - fail NotImplementedError + raise NotImplementedError end private @@ -116,35 +116,33 @@ def acquire_lock(token = nil) end def get - fail NotImplementedError + raise NotImplementedError end - def set(newval, cas) # rubocop:disable Lint/UnusedMethodArgument - fail NotImplementedError + def set(newval, cas) + raise NotImplementedError end - def initial_set(val = BLANK_STR) # rubocop:disable Lint/UnusedMethodArgument - fail NotImplementedError + def initial_set(val = BLANK_STR) + raise NotImplementedError end - def synchronize - mon_synchronize { yield } + def synchronize(&block) + mon_synchronize(&block) end - def retry_with_timeout + def retry_with_timeout(&block) start = Process.clock_gettime(Process::CLOCK_MONOTONIC) retry_count.times do elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start break if elapsed >= options[:acquisition_timeout] - synchronize do - yield - end + synchronize(&block) sleep(rand(options[:acquisition_delay] * 1000).to_f / 1000) end - rescue => _ + rescue StandardError => _e raise LockClientError end @@ -157,12 +155,12 @@ def deserialize_and_clear_locks(val) end def deserialize_locks(val) - unpacked = (val.nil? || val == BLANK_STR) ? [] : MessagePack.unpack(val) + unpacked = val.nil? || val == BLANK_STR ? [] : MessagePack.unpack(val) unpacked.map do |time, token| [Time.at(time), token] end - rescue EOFError, MessagePack::MalformedFormatError => _ + rescue EOFError, MessagePack::MalformedFormatError => _e [] end diff --git a/lib/suo/client/memcached.rb b/lib/suo/client/memcached.rb index dbc74b8..5e01f8c 100644 --- a/lib/suo/client/memcached.rb +++ b/lib/suo/client/memcached.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Suo module Client class Memcached < Base diff --git a/lib/suo/client/redis.rb b/lib/suo/client/redis.rb index 2a106c7..1b2eb28 100644 --- a/lib/suo/client/redis.rb +++ b/lib/suo/client/redis.rb @@ -1,7 +1,9 @@ +# frozen_string_literal: true + module Suo module Client class Redis < Base - OK_STR = "OK".freeze + OK_STR = "OK" def initialize(key, options = {}) options[:client] ||= ::Redis.new(options[:connection] || {}) @@ -40,10 +42,10 @@ def set(newval, _, expire: false) ret && ret[0] == OK_STR end - def synchronize - with { |r| r.watch(@key) { yield } } + def synchronize(&block) + with { |r| r.watch(@key, &block) } ensure - with { |r| r.unwatch } + with(&:unwatch) end def initial_set(val = BLANK_STR) diff --git a/lib/suo/errors.rb b/lib/suo/errors.rb index f7f2331..dfdb1b9 100644 --- a/lib/suo/errors.rb +++ b/lib/suo/errors.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Suo class LockClientError < StandardError; end end diff --git a/lib/suo/version.rb b/lib/suo/version.rb index 483521a..371cf72 100644 --- a/lib/suo/version.rb +++ b/lib/suo/version.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Suo - VERSION = "0.4.0".freeze + VERSION = "0.4.0" end diff --git a/suo.gemspec b/suo.gemspec index 5f23dd0..2107f8e 100644 --- a/suo.gemspec +++ b/suo.gemspec @@ -1,5 +1,6 @@ -# coding: utf-8 -lib = File.expand_path("../lib", __FILE__) +# frozen_string_literal: true + +lib = File.expand_path("lib", __dir__) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require "suo/version" @@ -16,18 +17,13 @@ Gem::Specification.new do |spec| spec.files = `git ls-files -z`.split("\x0") spec.bindir = "bin" - spec.test_files = spec.files.grep(%r{^(test|spec|features)/}) spec.require_paths = ["lib"] spec.required_ruby_version = ">= 3.1" spec.add_dependency "dalli" - spec.add_dependency "redis" spec.add_dependency "msgpack" + spec.add_dependency "redis" - spec.add_development_dependency "bundler" - spec.add_development_dependency "rake", "~> 13.0" - spec.add_development_dependency "rubocop", "~> 1.68.0" - spec.add_development_dependency "minitest", "~> 5.25" - spec.add_development_dependency "codeclimate-test-reporter", "~> 0.4.7" + spec.metadata["rubygems_mfa_required"] = "true" end diff --git a/test/client_test.rb b/test/client_test.rb index 8b7b3ed..2472639 100644 --- a/test/client_test.rb +++ b/test/client_test.rb @@ -1,6 +1,8 @@ +# frozen_string_literal: true + require "test_helper" -TEST_KEY = "suo_test_key".freeze +TEST_KEY = "suo_test_key" module ClientTests def client(options = {}) @@ -32,7 +34,7 @@ def test_single_resource_locking end def test_lock_with_custom_token - token = 'foo-bar' + token = "foo-bar" lock = @client.lock token assert_equal lock, token end @@ -86,7 +88,7 @@ def test_block_single_resource_locking def test_block_unlocks_on_exception assert_raises(RuntimeError) do - @client.lock{ fail "Test" } + @client.lock { raise "Test" } end assert_equal false, @client.locked? @@ -97,8 +99,18 @@ def test_readme_example @client = client(resources: 2) threads = [] - threads << Thread.new { @client.lock { output << "One"; sleep 0.5 } } - threads << Thread.new { @client.lock { output << "Two"; sleep 0.5 } } + threads << Thread.new do + @client.lock do + output << "One" + sleep 0.5 + end + end + threads << Thread.new do + @client.lock do + output << "Two" + sleep 0.5 + end + end sleep 0.1 threads << Thread.new { @client.lock { output << "Three" } } @@ -106,13 +118,13 @@ def test_readme_example ret = [] - ret << (output.size > 0 ? output.pop : nil) - ret << (output.size > 0 ? output.pop : nil) + ret << (output.size.positive? ? output.pop : nil) + ret << (output.size.positive? ? output.pop : nil) ret.sort! assert_equal 0, output.size - assert_equal %w(One Two), ret + assert_equal %w[One Two], ret assert_equal false, @client.locked? end @@ -166,7 +178,12 @@ def test_unstale_lock_acquisition @client = client(stale_lock_expiration: 0.5) - t1 = Thread.new { @client.lock { sleep 0.6; success_counter << 1 } } + t1 = Thread.new do + @client.lock do + sleep 0.6 + success_counter << 1 + end + end sleep 0.3 t2 = Thread.new do locked = @client.lock { success_counter << 1 } @@ -186,7 +203,12 @@ def test_stale_lock_acquisition @client = client(stale_lock_expiration: 0.5) - t1 = Thread.new { @client.lock { sleep 0.6; success_counter << 1 } } + t1 = Thread.new do + @client.lock do + sleep 0.6 + success_counter << 1 + end + end sleep 0.55 t2 = Thread.new do locked = @client.lock { success_counter << 1 } @@ -326,7 +348,7 @@ def test_increment_new_client threads = 2.times.map do Thread.new do - # note this is the method that generates a *new* client + # NOTE: this is the method that generates a *new* client client.lock { i += 1 } end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 403467f..2ff9f07 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,4 +1,6 @@ -$LOAD_PATH.unshift File.expand_path("../../lib", __FILE__) +# frozen_string_literal: true + +$LOAD_PATH.unshift File.expand_path("../lib", __dir__) if ENV["CODECLIMATE_REPO_TOKEN"] require "codeclimate-test-reporter" @@ -6,7 +8,6 @@ end require "suo" -require "thread" require "minitest/autorun" require "minitest/benchmark"