From 2c5bffcb6ac9ade5df29db36aeec64f88cbb5f16 Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Fri, 10 May 2024 18:08:56 +0100 Subject: [PATCH] Refactor repository and registry cache - Remove registry cache in the same engine that it is created - Get repository and registry cache paths in one place to avoid duplication --- .../lib/instance_verification/engine.rb | 53 ++++++++++++------- .../v3/systems/products_controller_spec.rb | 4 +- engines/registry/lib/registry/engine.rb | 10 +--- .../v3/systems/activations_controller_spec.rb | 15 +++--- .../v3/systems/products_controller_spec.rb | 16 +++--- 5 files changed, 53 insertions(+), 45 deletions(-) diff --git a/engines/instance_verification/lib/instance_verification/engine.rb b/engines/instance_verification/lib/instance_verification/engine.rb index 030668146..65c82eef9 100644 --- a/engines/instance_verification/lib/instance_verification/engine.rb +++ b/engines/instance_verification/lib/instance_verification/engine.rb @@ -5,21 +5,46 @@ def self.update_cache(remote_ip, system_login, product_id, is_byos: false, regis # TODO: BYOS scenario # to be addressed on a different PR unless registry - InstanceVerification.write_cache_file( - Rails.application.config.repo_cache_dir, - InstanceVerification.build_cache_key(remote_ip, system_login, base_product_id: product_id) - ) + # write repository cache + repository_cache_path = InstanceVerification.repository_cache_path(remote_ip, system_login, product_id) + InstanceVerification.write_cache_file(repository_cache_path) unless registry end - InstanceVerification.write_cache_file( + # write registry cache + registry_cache_path = InstanceVerification.registry_cache_path(remote_ip, system_login) + InstanceVerification.write_cache_file(registry_cache_path) + end + + def self.write_cache_file(cache_path) + Dir.mkdir(File.dirname(cache_path)) unless File.directory?(File.dirname(cache_path)) + + FileUtils.touch(cache_path) + end + + def self.repository_cache_path(remote_ip, system_login, product_id) + File.join( + Rails.application.config.repo_cache_dir, + InstanceVerification.build_cache_key(remote_ip, system_login, product_id: product_id) + ) + end + + def self.registry_cache_path(remote_ip, system_login) + File.join( Rails.application.config.registry_cache_dir, InstanceVerification.build_cache_key(remote_ip, system_login) ) end - def self.write_cache_file(cache_dir, cache_key) - FileUtils.mkdir_p(cache_dir) - FileUtils.touch(File.join(cache_dir, cache_key)) + def self.remove_registry_cache(remote_ip, system_login) + registry_cache_path = InstanceVerification.registry_cache_path(remote_ip, system_login) + File.unlink(registry_cache_path) if File.exist?(registry_cache_path) + end + + def self.build_cache_key(remote_ip, system_login, product_id: nil) + cache_key = [remote_ip, system_login] + cache_key.append(product_id) unless product_id.nil? + + cache_key.join('-') end def self.verify_instance(request, logger, system) @@ -30,10 +55,7 @@ def self.verify_instance(request, logger, system) return false unless base_product # check the cache for the system (20 min) - cache_path = File.join( - Rails.application.config.repo_cache_dir, - InstanceVerification.build_cache_key(request.remote_ip, system.login, base_product_id: base_product.id) - ) + cache_path = InstanceVerification.repository_cache_path(request.remote_ip, system.login, base_product.id) if File.exist?(cache_path) # only update registry cache key InstanceVerification.update_cache(request.remote_ip, system.login, nil, is_byos: system.proxy_byos, registry: true) @@ -82,13 +104,6 @@ def self.verify_instance(request, logger, system) false end - def self.build_cache_key(remote_ip, login, base_product_id: nil) - cache_key = [remote_ip, login] - cache_key.append(base_product_id) unless base_product_id.nil? - - cache_key.join('-') - end - class Engine < ::Rails::Engine isolate_namespace InstanceVerification config.generators.api_only = true diff --git a/engines/instance_verification/spec/requests/api/connect/v3/systems/products_controller_spec.rb b/engines/instance_verification/spec/requests/api/connect/v3/systems/products_controller_spec.rb index a8cc64bcc..5b9701abd 100644 --- a/engines/instance_verification/spec/requests/api/connect/v3/systems/products_controller_spec.rb +++ b/engines/instance_verification/spec/requests/api/connect/v3/systems/products_controller_spec.rb @@ -116,8 +116,8 @@ allow(File).to receive(:directory?) allow(Dir).to receive(:mkdir) allow(FileUtils).to receive(:touch) - expect(InstanceVerification).to receive(:write_cache_file).once.with('repo/cache', "127.0.0.1-#{system.login}-#{product_sap.id}") - expect(InstanceVerification).to receive(:write_cache_file).once.with('registry/cache', "127.0.0.1-#{system.login}") + expect(InstanceVerification).to receive(:write_cache_file).once.with("repo/cache/127.0.0.1-#{system.login}-#{product_sap.id}") + expect(InstanceVerification).to receive(:write_cache_file).once.with("registry/cache/127.0.0.1-#{system.login}") post url, params: payload_sap, headers: headers end diff --git a/engines/registry/lib/registry/engine.rb b/engines/registry/lib/registry/engine.rb index 687212464..570605428 100644 --- a/engines/registry/lib/registry/engine.rb +++ b/engines/registry/lib/registry/engine.rb @@ -1,11 +1,4 @@ module Registry - class << self - def remove_auth_cache(registry_cache_key) - cache_path = File.join(Rails.application.config.registry_cache_dir, registry_cache_key) - File.unlink(registry_cache_path) if File.exist?(cache_path) - end - end - class Engine < ::Rails::Engine isolate_namespace Registry config.generators.api_only = true @@ -25,8 +18,7 @@ def handle_auth_cache before_action :remove_auth_cache, only: %w[deregister] def remove_auth_cache - registry_cache_key = [request.remote_ip, @system.login].join('-') - Registry.remove_auth_cache(registry_cache_key) + InstanceVerification.remove_registry_cache(request.remote_ip, @system.login) end end end diff --git a/engines/registry/spec/requests/api/connect/v3/systems/activations_controller_spec.rb b/engines/registry/spec/requests/api/connect/v3/systems/activations_controller_spec.rb index 57a0faf5e..c85b360c1 100644 --- a/engines/registry/spec/requests/api/connect/v3/systems/activations_controller_spec.rb +++ b/engines/registry/spec/requests/api/connect/v3/systems/activations_controller_spec.rb @@ -26,21 +26,22 @@ before do allow(File).to receive(:join).and_call_original - # allow(File).to receive(:exist?).with('repo/cache/127.0.0.1-login45-1052122') - # allow(File).to receive(:exist?).with("repo/cache/127.0.0.1-#{system.login}-#{system.products.first.id}").and_return(true) - # allow(File).to receive(:exist?).with("repo/cache/127.0.0.1-#{system.login}-#{system.products.first.id}").and_return(true) - # allow(File).to receive(:exist?) + allow(File).to receive(:directory?) + allow(Dir).to receive(:mkdir) + allow(FileUtils).to receive(:touch) + allow(InstanceVerification).to receive(:repository_cache_path).and_return('.') + allow(InstanceVerification).to receive(:registry_cache_path) + allow(InstanceVerification).to receive(:write_cache_file) allow(InstanceVerification).to receive(:update_cache) allow(InstanceVerification).to receive(:verify_instance).and_call_original headers['X-Instance-Data'] = 'IMDS' end it 'refreshes registry cache key only' do - FileUtils.mkdir_p('repo/cache') - FileUtils.touch(cache_name) + allow(File).to receive(:directory?) + allow(Dir).to receive(:mkdir) expect(InstanceVerification).to receive(:update_cache).with('127.0.0.1', system.login, nil, is_byos: system.proxy_byos, registry: true) get '/connect/systems/activations', headers: headers - FileUtils.rm_rf('repo/cache') data = JSON.parse(response.body) expect(data[0]['service']['url']).to match(%r{^plugin:/susecloud}) end diff --git a/engines/scc_proxy/spec/requests/api/connect/v3/systems/products_controller_spec.rb b/engines/scc_proxy/spec/requests/api/connect/v3/systems/products_controller_spec.rb index b0ef27a1f..bc2d1ff76 100644 --- a/engines/scc_proxy/spec/requests/api/connect/v3/systems/products_controller_spec.rb +++ b/engines/scc_proxy/spec/requests/api/connect/v3/systems/products_controller_spec.rb @@ -114,8 +114,8 @@ allow(FileUtils).to receive(:mkdir_p) allow(FileUtils).to receive(:touch) - allow(InstanceVerification).to receive(:write_cache_file).twice.with('repo/cache', "127.0.0.1-#{system.login}-#{product.id}") - allow(InstanceVerification).to receive(:write_cache_file).twice.with('registry/cache', "127.0.0.1-#{system.login}") + allow(InstanceVerification).to receive(:write_cache_file).twice.with("repo/cache/127.0.0.1-#{system.login}-#{product.id}") + allow(InstanceVerification).to receive(:write_cache_file).twice.with("registry/cache/127.0.0.1-#{system.login}") end it 'renders service JSON' do @@ -132,8 +132,8 @@ body: '{"error": "No product found on SCC for: foo bar x86_64 json api"}', headers: {} ) - allow(InstanceVerification).to receive(:write_cache_file).twice.with('repo/cache', "127.0.0.1-#{system.login}-#{product.id}") - allow(InstanceVerification).to receive(:write_cache_file).twice.with('registry/cache', "127.0.0.1-#{system.login}") + allow(InstanceVerification).to receive(:write_cache_file).twice.with("repo/cache/127.0.0.1-#{system.login}-#{product.id}") + allow(InstanceVerification).to receive(:write_cache_file).twice.with("registry/cache/127.0.0.1-#{system.login}") allow(FileUtils).to receive(:mkdir_p) allow(FileUtils).to receive(:touch) @@ -165,8 +165,8 @@ body: '{"id": "bar"}', headers: {} ) - allow(InstanceVerification).to receive(:write_cache_file).twice.with('repo/cache', "127.0.0.1-#{system.login}-#{product.id}") - allow(InstanceVerification).to receive(:write_cache_file).twice.with('registry/cache', "127.0.0.1-#{system.login}") + allow(InstanceVerification).to receive(:write_cache_file).twice.with("repo/cache/127.0.0.1-#{system.login}-#{product.id}") + allow(InstanceVerification).to receive(:write_cache_file).twice.with("registry/cache/127.0.0.1-#{system.login}") allow(File).to receive(:directory?) allow(FileUtils).to receive(:mkdir_p) allow(FileUtils).to receive(:touch) @@ -201,8 +201,8 @@ headers: {} ) - allow(InstanceVerification).to receive(:write_cache_file).twice.with('repo/cache', "127.0.0.1-#{system.login}-#{product.id}") - allow(InstanceVerification).to receive(:write_cache_file).twice.with('registry/cache', "127.0.0.1-#{system.login}") + allow(InstanceVerification).to receive(:write_cache_file).twice.with("repo/cache/127.0.0.1-#{system.login}-#{product.id}") + allow(InstanceVerification).to receive(:write_cache_file).twice.with("registry/cache/127.0.0.1-#{system.login}") allow(File).to receive(:directory?) allow(FileUtils).to receive(:mkdir_p) allow(FileUtils).to receive(:touch)