Merge pull request #515 from Shopify/update-rubocop-shopify

Update rubocop-shopify and correct offenses

Author: dylanahsmith

Gemfile

@@ -1,12 +1,11 @@
 # frozen_string_literal: true
+
 source "https://rubygems.org"
 gemspec
 
-# Skip rubocop 1.16.1 until the next release which will include the fix:
-# https://github.com/rubocop/rubocop/pull/9862
-gem "rubocop", "~> 1.5", "!= 1.16.1"
+gem "rubocop", "~> 1.5"
 
-gem "rubocop-shopify", "~> 2.0.1", require: false
+gem "rubocop-shopify", "~> 2.6.0", require: false
 
 gem "mysql2", "~> 0.5.3", platform: :mri
 gem "pg", ">= 0.18", "< 2.0", platform: :mri

Rakefile

@@ -1,5 +1,6 @@
 #!/usr/bin/env rake
 # frozen_string_literal: true
+
 require "bundler/gem_tasks"
 
 require "rake/testtask"
@@ -23,7 +24,7 @@ end
 
 desc("Update serialization format test fixture.")
 task :update_serialization_format do
-  %w(mysql2 postgresql).each do |db|
+  ["mysql2", "postgresql"].each do |db|
     ENV["DB"] = db
     ruby "./test/helpers/update_serialization_format.rb"
   end

gemfiles/Gemfile.latest-release

@@ -1,10 +1,8 @@
 source "https://rubygems.org"
 gemspec path: ".."
 
-# Skip rubocop 1.16.1 until the next release which will include the fix:
-# https://github.com/rubocop/rubocop/pull/9862
-gem "rubocop", "~> 1.5", "!= 1.16.1"
-gem "rubocop-shopify", "~> 2.0.1", require: false
+gem "rubocop", "~> 1.5"
+gem "rubocop-shopify", "~> 2.6.0", require: false
 
 gem "activerecord"
 gem "activesupport"

identity_cache.gemspec

@@ -1,5 +1,6 @@
 # -*- encoding: utf-8 -*-
 # frozen_string_literal: true
+
 require File.expand_path("../lib/identity_cache/version", __FILE__)
 
 Gem::Specification.new do |gem|
@@ -15,11 +16,11 @@ Gem::Specification.new do |gem|
   gem.email         = ["gems@shopify.com"]
   gem.description   = "Opt-in read through Active Record caching."
   gem.summary       = "IdentityCache lets you specify how you want to cache your " \
-                      "model objects, at the model level, and adds a number of " \
-                      "convenience methods for accessing those objects through " \
-                      "the cache. Memcached is used as the backend cache store, " \
-                      "and the database is only hit when a copy of the object " \
-                      "cannot be found in Memcached."
+    "model objects, at the model level, and adds a number of " \
+    "convenience methods for accessing those objects through " \
+    "the cache. Memcached is used as the backend cache store, " \
+    "and the database is only hit when a copy of the object " \
+    "cannot be found in Memcached."
   gem.homepage      = "https://github.com/Shopify/identity_cache"
 
   gem.files         = Dir.chdir(File.expand_path(__dir__)) do
@@ -35,11 +36,11 @@ Gem::Specification.new do |gem|
 
   gem.metadata["allowed_push_host"] = "https://rubygems.org"
 
-  gem.add_dependency("ar_transaction_changes", "~> 1.1")
   gem.add_dependency("activerecord", ">= 5.2")
+  gem.add_dependency("ar_transaction_changes", "~> 1.1")
 
-  gem.add_development_dependency("rake", "~> 13.0")
+  gem.add_development_dependency("minitest", "~> 5.14")
   gem.add_development_dependency("mocha", "~> 1.12")
+  gem.add_development_dependency("rake", "~> 13.0")
   gem.add_development_dependency("spy", "~> 1.0")
-  gem.add_development_dependency("minitest", "~> 5.14")
 end

lib/identity_cache.rb

@@ -1,4 +1,5 @@
 # frozen_string_literal: true
+
 require "active_record"
 require "active_support/core_ext/module/attribute_accessors"
 require "ar_transaction_changes"
@@ -84,6 +85,7 @@ class << self
 
     def append_features(base) # :nodoc:
       raise AlreadyIncludedError if base.include?(IdentityCache)
+
       super
     end
 
@@ -200,6 +202,7 @@ def with_fetch_read_only_records(value = true)
     def fetch_read_only_records
       v = Thread.current[:identity_cache_fetch_read_only_records]
       return v unless v.nil?
+
       @fetch_read_only_records
     end
 
@@ -209,9 +212,9 @@ def eager_load!
 
     private
 
-    def fetch_in_batches(keys)
+    def fetch_in_batches(keys, &block)
       keys.each_slice(BATCH_SIZE).each_with_object({}) do |slice, result|
-        result.merge!(cache.fetch_multi(*slice) { |missed_keys| yield missed_keys })
+        result.merge!(cache.fetch_multi(*slice, &block))
       end
     end
   end

lib/identity_cache/belongs_to_caching.rb

@@ -1,4 +1,5 @@
 # frozen_string_literal: true
+
 module IdentityCache
   module BelongsToCaching
     extend ActiveSupport::Concern

lib/identity_cache/cache_fetcher.rb

@@ -15,6 +15,7 @@ class FillLock
       class << self
         def from_cache(marker, client_id, data_version)
           raise ArgumentError unless marker == FILL_LOCKED
+
           new(client_id: client_id, data_version: data_version)
         end
 
@@ -69,13 +70,11 @@ def fetch_multi(keys, &block)
       results
     end
 
-    def fetch(key, fill_lock_duration: nil, lock_wait_tries: 2)
+    def fetch(key, fill_lock_duration: nil, lock_wait_tries: 2, &block)
       if fill_lock_duration && IdentityCache.should_fill_cache?
-        fetch_with_fill_lock(key, fill_lock_duration, lock_wait_tries) do
-          yield
-        end
+        fetch_with_fill_lock(key, fill_lock_duration, lock_wait_tries, &block)
       else
-        fetch_without_fill_lock(key) { yield }
+        fetch_without_fill_lock(key, &block)
       end
     end
 
@@ -88,16 +87,19 @@ def fetch_without_fill_lock(key)
         unless value.nil?
           return value
         end
+
         data = yield
         break unless IdentityCache.should_fill_cache?
+
         data
       end
       data
     end
 
-    def fetch_with_fill_lock(key, fill_lock_duration, lock_wait_tries)
+    def fetch_with_fill_lock(key, fill_lock_duration, lock_wait_tries, &block)
       raise ArgumentError, "fill_lock_duration must be greater than 0.0" unless fill_lock_duration > 0.0
       raise ArgumentError, "lock_wait_tries must be greater than 0" unless lock_wait_tries > 0
+
       lock = nil
       using_fallback_key = false
       expiration_options = EMPTY_HASH
@@ -122,13 +124,14 @@ def fetch_with_fill_lock(key, fill_lock_duration, lock_wait_tries)
             return data
           else
             raise LockWaitTimeout if lock_wait_tries <= 0
+
             lock_wait_tries -= 1
 
             # If fill failed in the other client, then it might be failing fast
             # so avoid waiting the typical amount of time for a lock wait. The
             # semian gem can be used to handle failing fast when the database is slow.
             if lock.fill_failed?
-              return fetch_without_fill_lock(key) { yield }
+              return fetch_without_fill_lock(key, &block)
             end
 
             # lock wait
@@ -167,8 +170,10 @@ def fetch_with_fill_lock(key, fill_lock_duration, lock_wait_tries)
     def mark_fill_failure_on_lock(key, expiration_options)
       @cache_backend.cas(key, expiration_options) do |value|
         break unless FillLock.cache_value?(value)
+
         lock = FillLock.from_cache(*value)
         break if lock.client_id != client_id
+
         lock.mark_failed
         lock.cache_value
       end
@@ -229,10 +234,12 @@ def fill_with_lock(key, data, my_lock, expiration_options)
       upserted = upsert(key, expiration_options) do |value|
         return false if value.nil? || value == IdentityCache::DELETED
         return true unless FillLock.cache_value?(value) # already filled
+
         current_lock = FillLock.from_cache(*value)
         if current_lock.data_version != my_lock.data_version
           return false # invalidated then relocked
         end
+
         data
       end
 
@@ -285,6 +292,7 @@ def cas_multi(keys)
 
         break if updates.empty?
         break unless IdentityCache.should_fill_cache?
+
         updates
       end
       result
@@ -298,6 +306,7 @@ def add_multi(keys)
 
     def add(key, value, expiration_options = EMPTY_HASH)
       return false unless IdentityCache.should_fill_cache?
+
       @cache_backend.write(key, value, { unless_exist: true, **expiration_options })
     end
   end

lib/identity_cache/cache_hash.rb

@@ -1,4 +1,5 @@
 # frozen_string_literal: true
+
 # Use CityHash for fast hashing if it is available; use Digest::MD5 otherwise
 begin
   require "cityhash"

lib/identity_cache/cache_invalidation.rb

@@ -1,4 +1,5 @@
 # frozen_string_literal: true
+
 module IdentityCache
   module CacheInvalidation
     CACHE_KEY_NAMES = [:ids_variable_name, :id_variable_name, :records_variable_name]

lib/identity_cache/cache_key_generation.rb

@@ -1,4 +1,5 @@
 # frozen_string_literal: true
+
 module IdentityCache
   module CacheKeyGeneration
     extend ActiveSupport::Concern