Merge branch 'master' into 78798-add-brd-test

Gemfile

@@ -174,7 +174,7 @@ end
 
 group :test do
   gem 'apivore', git: 'https://github.com/department-of-veterans-affairs/apivore', tag: 'v2.0.0.vsp'
-  gem 'fakeredis'
+  gem 'mock_redis'
   gem 'pdf-inspector'
   gem 'rspec_junit_formatter'
   gem 'rspec-retry'
@@ -215,7 +215,7 @@ group :development, :test do
   gem 'rubocop-rails'
   gem 'rubocop-rspec'
   gem 'rubocop-thread_safety'
-  gem 'sidekiq', '>= 6.4.0'
+  gem 'sidekiq', '~> 7.2.0'
   gem 'timecop'
   gem 'webmock'
   gem 'yard'

Gemfile.lock

@@ -133,12 +133,13 @@ PATH
 GEM
   remote: https://enterprise.contribsys.com/
   specs:
-    sidekiq-ent (2.5.3)
-      einhorn (>= 0.7.4)
-      sidekiq (>= 6.5.0, < 7)
-      sidekiq-pro (>= 5.5.0, < 6)
-    sidekiq-pro (5.5.8)
-      sidekiq (~> 6.0, >= 6.5.6)
+    sidekiq-ent (7.2.2)
+      einhorn (~> 1.0)
+      gserver
+      sidekiq (>= 7.2.0, < 8)
+      sidekiq-pro (>= 7.2.0, < 8)
+    sidekiq-pro (7.2.0)
+      sidekiq (>= 7.2.0, < 8)
 
 GEM
   remote: https://rubygems.org/
@@ -417,8 +418,6 @@ GEM
       railties (>= 5.0.0)
     faker (3.2.3)
       i18n (>= 1.8.11, < 2)
-    fakeredis (0.9.2)
-      redis (~> 4.8)
     faraday (2.9.0)
       faraday-net_http (>= 2.0, < 3.2)
     faraday-follow_redirects (0.3.0)
@@ -634,6 +633,7 @@ GEM
     mini_mime (1.1.5)
     mini_portile2 (2.8.5)
     minitest (5.22.3)
+    mock_redis (0.44.0)
     msgpack (1.7.2)
     msgpack (1.7.2-java)
     multi_json (1.15.0)
@@ -812,7 +812,10 @@ GEM
     rchardet (1.8.0)
     rdoc (6.6.2)
       psych (>= 4.0.0)
-    redis (4.8.1)
+    redis (5.1.0)
+      redis-client (>= 0.17.0)
+    redis-client (0.20.0)
+      connection_pool
     redis-namespace (1.11.0)
       redis (>= 4)
     regexp_parser (2.9.0)
@@ -904,7 +907,7 @@ GEM
     rubocop-factory_bot (2.25.1)
       rubocop (~> 1.41)
     rubocop-junit-formatter (0.1.4)
-    rubocop-rails (2.24.0)
+    rubocop-rails (2.24.1)
       activesupport (>= 4.2.0)
       rack (>= 1.1)
       rubocop (>= 1.33.0, < 2.0)
@@ -954,10 +957,11 @@ GEM
     shrine (3.5.0)
       content_disposition (~> 1.0)
       down (~> 5.1)
-    sidekiq (6.5.12)
-      connection_pool (>= 2.2.5, < 3)
-      rack (~> 2.0)
-      redis (>= 4.5.0, < 5)
+    sidekiq (7.2.2)
+      concurrent-ruby (< 2)
+      connection_pool (>= 2.3.0)
+      rack (>= 2.2.4)
+      redis-client (>= 0.19.0)
     sidekiq_alive (2.4.0)
       gserver (~> 0.0.1)
       sidekiq (>= 5, < 8)
@@ -1122,7 +1126,6 @@ DEPENDENCIES
   facilities_api!
   factory_bot_rails
   faker
-  fakeredis
   faraday (~> 2.9)
   faraday-follow_redirects
   faraday-httpclient
@@ -1170,6 +1173,7 @@ DEPENDENCIES
   mimemagic
   mini_magick
   mobile!
+  mock_redis
   mocked_authentication!
   my_health!
   net-sftp
@@ -1234,7 +1238,7 @@ DEPENDENCIES
   sentry-ruby
   shoulda-matchers
   shrine
-  sidekiq (>= 6.4.0)
+  sidekiq (~> 7.2.0)
   sidekiq-ent!
   sidekiq-pro!
   sidekiq_alive

config/initializers/01_redis.rb

@@ -4,6 +4,10 @@
 REDIS_CONFIG = Rails.application.config_for(:redis).freeze
 # set the current global instance of Redis based on environment specific config
 
-$redis = Redis.new(REDIS_CONFIG[:redis].to_hash)
-
-Redis.exists_returns_integer = true
+$redis =
+  if Rails.env.test?
+    require 'mock_redis'
+    MockRedis.new(url: REDIS_CONFIG[:redis][:url])
+  else
+    Redis.new(REDIS_CONFIG[:redis].to_h)
+  end

config/initializers/sidekiq.rb

@@ -59,6 +59,8 @@
     end
 
     # Remove the default error handler
-    config.error_handlers.delete_if { |handler| handler.is_a?(Sidekiq::ExceptionHandler::Logger) }
+    config.error_handlers.delete(Sidekiq::Config::ERROR_HANDLER)
   end
+
+  Sidekiq.strict_args!(false)
 end

config/redis.yml

@@ -197,6 +197,9 @@ test:
   <<: *defaults
   redis:
     inherit_socket: true
+    url: <%= Settings.redis.app_data.url %>
+  sidekiq:
+    url: <%= Settings.redis.sidekiq.url %>
 
 production:
   <<: *defaults

docker-compose.test.yml

@@ -1,5 +1,7 @@
 version: '3.4'
 services:
+  redis:
+    image: redis:6.2-alpine
   postgres:
     image: mdillon/postgis:11-alpine
     command: postgres -c shared_preload_libraries=pg_stat_statements -c pg_stat_statements.track=all -c max_connections=200
@@ -21,6 +23,8 @@ services:
       "Settings.database_url": "postgis://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-password}@${POSTGRES_HOST:-postgres}:${POSTGRES_PORT:-5432}/${POSTGRES_DATABASE:-vets_api_development}?pool=4"
       "Settings.test_database_url": "postgis://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-password}@${POSTGRES_HOST:-postgres}:${POSTGRES_PORT:-5432}/${POSTGRES_DATABASE:-vets_api_test}"
       "Settings.binaries.clamdscan": "clamscan"           # Not running a separate process within the container for clamdscan, so we use clamscan which requires no daemon
+      "Settings.redis.app_data.url": "redis://redis:6379"
+      "Settings.redis.sidekiq.url": "redis://redis:6379"
       POSTGRES_HOST: "${POSTGRES_HOST:-postgres}"
       POSTGRES_PORT: "${POSTGRES_PORT:-5432}"
       POSTGRES_USER: "${POSTGRES_USER:-postgres}"
@@ -34,7 +38,9 @@ services:
       DANGER_GITHUB_API_TOKEN: "${DANGER_GITHUB_API_TOKEN}"
     depends_on:
       - postgres
+      - redis
     links:
       - postgres
+      - redis
 volumes:
   test_bundle:

lib/common/models/redis_store.rb

@@ -48,6 +48,8 @@ def initialize(attributes = {}, persisted = false)
     end
 
     def self.find(redis_key = nil)
+      return nil if redis_key.nil?
+
       response = redis_namespace.get(redis_key)
       return nil unless response
 

lib/sidekiq/semantic_logging.rb

@@ -4,6 +4,11 @@
 require 'sidekiq/job_logger'
 
 class Sidekiq::SemanticLogging < Sidekiq::JobLogger
+  def initialize
+    logger = Rails.logger
+    super(logger)
+  end
+
   def call(_worker, item, queue)
     logger_tags = {
       class: item['class'],

modules/appeals_api/config/mailinglists/report_daily.yml

@@ -21,5 +21,6 @@ production:
   - [email protected]
   - [email protected]
   - [email protected]
+  - [email protected]
   - [email protected]
   - [email protected]

modules/ask_va_api/app/services/ask_va_api/redis_client.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module AskVAApi
+  class RedisClient
+    def fetch(key)
+      Rails.cache.read(
+        key,
+        namespace: 'crm-api-cache'
+      )
+    end
+
+    def store_data(key:, data:, ttl:)
+      Rails.cache.write(
+        key,
+        data,
+        namespace: 'crm-api-cache',
+        expires_in: ttl
+      )
+    end
+  end
+end

modules/ask_va_api/app/services/crm/cache_data.rb

@@ -4,7 +4,7 @@ module Crm
   class CacheData
     attr_reader :cache_client, :service
 
-    def initialize(service: Service.new(icn: nil), cache_client: RedisClient.new)
+    def initialize(service: Service.new(icn: nil), cache_client: AskVAApi::RedisClient.new)
       @cache_client = cache_client
       @service = service
     end

modules/ask_va_api/app/services/crm/crm_token.rb

@@ -16,7 +16,7 @@ class CrmToken
 
     def initialize
       @settings = Settings.ask_va_api.crm_api
-      @cache_client = RedisClient.new
+      @cache_client = AskVAApi::RedisClient.new
       @logger = LogService.new
     end
 

modules/ask_va_api/spec/services/crm/cache_data_spec.rb

@@ -4,7 +4,7 @@
 
 RSpec.describe Crm::CacheData do
   let(:service) { double('Crm::Service') }
-  let(:cache_client) { double('RedisClient') }
+  let(:cache_client) { double('AskVAApi::RedisClient') }
   let(:cache_data_instance) { Crm::CacheData.new(service:, cache_client:) }
   let(:cache_data) { { topics: [{ id: 1, name: 'Topic 1' }] } }
 

modules/ask_va_api/spec/services/redis_client_spec.rb

@@ -2,8 +2,8 @@
 
 require 'rails_helper'
 
-RSpec.describe RedisClient do
-  let(:redis_client) { RedisClient.new }
+RSpec.describe AskVAApi::RedisClient do
+  let(:redis_client) { AskVAApi::RedisClient.new }
   let(:token) { 'some-access-token' }
 
   describe '#fetch' do

spec/lib/chip/redis_client_spec.rb

@@ -29,11 +29,17 @@
     let(:token) { 'test_token' }
 
     it 'saves entry' do
-      expect_any_instance_of(Redis).to receive(:set).once.with(
-        "chip:#{tenant_id}", 'test_token', { ex: REDIS_CONFIG[:chip][:each_ttl] }
+      expect_any_instance_of(Redis::Namespace).to receive(:set).once.with(
+        tenant_id, 'test_token', { ex: REDIS_CONFIG[:chip][:each_ttl] }
       )
       redis_client.save(token:)
     end
+
+    it 'saves entry with chip namespace' do
+      redis_client.save(token:)
+
+      expect(redis_client.redis_namespace.redis.keys).to include("chip:#{tenant_id}")
+    end
   end
 
   describe 'ttl' do

spec/lib/common/models/redis_store_spec.rb

@@ -62,12 +62,18 @@
 
   describe '#save' do
     it 'saves serialized class to redis with the correct namespace' do
-      expect_any_instance_of(Redis).to receive(:set).once.with(
-        'my_namespace:e66fd7b7-94e0-4748-8063-283f55efb0ea',
+      expect_any_instance_of(Redis::Namespace).to receive(:set).once.with(
+        'e66fd7b7-94e0-4748-8063-283f55efb0ea',
         '{":uuid":"e66fd7b7-94e0-4748-8063-283f55efb0ea",":email":"[email protected]"}'
       )
       subject.save
     end
+
+    it 'saves entry with namespace' do
+      subject.save
+
+      expect(subject.redis_namespace.redis.keys).to include('my_namespace:e66fd7b7-94e0-4748-8063-283f55efb0ea')
+    end
   end
 
   describe '#update' do
@@ -92,12 +98,18 @@
 
   describe '#destroy' do
     it 'removes itself from redis with the correct namespace' do
-      expect_any_instance_of(Redis).to receive(:del).once.with(
-        'my_namespace:e66fd7b7-94e0-4748-8063-283f55efb0ea'
+      expect_any_instance_of(Redis::Namespace).to receive(:del).once.with(
+        'e66fd7b7-94e0-4748-8063-283f55efb0ea'
       )
       subject.destroy
     end
 
+    it "entry doesn't exists" do
+      subject.destroy
+
+      expect(subject.redis_namespace.redis.keys).not_to include('my_namespace:e66fd7b7-94e0-4748-8063-283f55efb0ea')
+    end
+
     it 'freezes the instance after destroy is called' do
       subject.destroy
       expect(subject.destroyed?).to eq(true)

spec/requests/breakers_integration_spec.rb

@@ -30,7 +30,6 @@
     # Not clearing the breakers would cause subsequent RX calls to fail after the breaker is
     # triggered in this group.
 
-    # fakeredis/rspec has a `before` callback, but it's for the suite, not each example. Oops.
     Breakers.client.redis_connection.redis.flushdb
   end
 

spec/spec_helper.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 
-require 'fakeredis/rspec'
 require 'i18n'
 require 'support/spec_builders'
 require 'support/matchers'
@@ -15,6 +14,7 @@
 require 'rspec/its'
 require 'rspec/retry'
 require 'aasm/rspec'
+require 'mock_redis'
 
 # By default run SimpleCov, but allow an environment variable to disable.
 unless ENV['NOCOVERAGE']
@@ -192,4 +192,8 @@
   config.after do
     Timecop.return
   end
+
+  config.before do
+    $redis.flushdb
+  end
 end