diff --git a/Gemfile b/Gemfile index ca26ce94c50..056e8a031aa 100644 --- a/Gemfile +++ b/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' diff --git a/Gemfile.lock b/Gemfile.lock index 66b5c15fe00..3dd7a71f6dd 100644 --- a/Gemfile.lock +++ b/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 diff --git a/config/initializers/01_redis.rb b/config/initializers/01_redis.rb index bd43c3a719d..fcf0ffd2bd5 100644 --- a/config/initializers/01_redis.rb +++ b/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 diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index b2852d54867..41fb273cc5c 100644 --- a/config/initializers/sidekiq.rb +++ b/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 diff --git a/config/redis.yml b/config/redis.yml index c3444f1575c..3cde49a7e0b 100644 --- a/config/redis.yml +++ b/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 diff --git a/docker-compose.test.yml b/docker-compose.test.yml index e9f5a783f9f..907cf5c2d27 100644 --- a/docker-compose.test.yml +++ b/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: diff --git a/lib/common/models/redis_store.rb b/lib/common/models/redis_store.rb index 755a92a5980..fa89590beb1 100644 --- a/lib/common/models/redis_store.rb +++ b/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 diff --git a/lib/sidekiq/semantic_logging.rb b/lib/sidekiq/semantic_logging.rb index 822699bd393..b6043e958ca 100644 --- a/lib/sidekiq/semantic_logging.rb +++ b/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'], diff --git a/modules/appeals_api/config/mailinglists/report_daily.yml b/modules/appeals_api/config/mailinglists/report_daily.yml index f1be8464b4a..a406f7074c4 100644 --- a/modules/appeals_api/config/mailinglists/report_daily.yml +++ b/modules/appeals_api/config/mailinglists/report_daily.yml @@ -21,5 +21,6 @@ production: - robin.garrison@adhocteam.us - rocio.de-santiago@coforma.io - sade@coforma.io + - shaun.burdick@coforma.io - steve.albers@va.gov - zachary.goldfine@va.gov diff --git a/modules/ask_va_api/app/services/ask_va_api/redis_client.rb b/modules/ask_va_api/app/services/ask_va_api/redis_client.rb new file mode 100644 index 00000000000..5c25c9f5c40 --- /dev/null +++ b/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 diff --git a/modules/ask_va_api/app/services/crm/cache_data.rb b/modules/ask_va_api/app/services/crm/cache_data.rb index a3d286b9f33..9b53c16c82e 100644 --- a/modules/ask_va_api/app/services/crm/cache_data.rb +++ b/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 diff --git a/modules/ask_va_api/app/services/crm/crm_token.rb b/modules/ask_va_api/app/services/crm/crm_token.rb index b945e181ed7..3b023fa2ced 100644 --- a/modules/ask_va_api/app/services/crm/crm_token.rb +++ b/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 diff --git a/modules/ask_va_api/app/services/redis_client.rb b/modules/ask_va_api/app/services/redis_client.rb deleted file mode 100644 index ae809c2eed2..00000000000 --- a/modules/ask_va_api/app/services/redis_client.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -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 diff --git a/modules/ask_va_api/spec/services/crm/cache_data_spec.rb b/modules/ask_va_api/spec/services/crm/cache_data_spec.rb index b1b247a2b5c..1e341f6469d 100644 --- a/modules/ask_va_api/spec/services/crm/cache_data_spec.rb +++ b/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' }] } } diff --git a/modules/ask_va_api/spec/services/redis_client_spec.rb b/modules/ask_va_api/spec/services/redis_client_spec.rb index fd58fdc3795..b8d2b7e74c0 100644 --- a/modules/ask_va_api/spec/services/redis_client_spec.rb +++ b/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 diff --git a/spec/lib/chip/redis_client_spec.rb b/spec/lib/chip/redis_client_spec.rb index cb8f8c9e618..6d5e4c7d667 100644 --- a/spec/lib/chip/redis_client_spec.rb +++ b/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 diff --git a/spec/lib/common/models/redis_store_spec.rb b/spec/lib/common/models/redis_store_spec.rb index d9147240c6b..0e0b3a1cfd0 100644 --- a/spec/lib/common/models/redis_store_spec.rb +++ b/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":"foo@bar.com"}' ) 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) diff --git a/spec/requests/breakers_integration_spec.rb b/spec/requests/breakers_integration_spec.rb index 48b56c4b60c..512c36ce557 100644 --- a/spec/requests/breakers_integration_spec.rb +++ b/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 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b858a23e81a..8b036e3f589 100644 --- a/spec/spec_helper.rb +++ b/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