diff --git a/app/controllers/v1/gids/lcpe/exams_controller.rb b/app/controllers/v1/gids/lcpe/exams_controller.rb index 9efac2bffe3..3298b05a490 100644 --- a/app/controllers/v1/gids/lcpe/exams_controller.rb +++ b/app/controllers/v1/gids/lcpe/exams_controller.rb @@ -4,8 +4,6 @@ module V1 module GIDS module LCPE class ExamsController < GIDS::LCPEController - VERSIONING_PARAMS = %i[version].freeze - def index exams = service.get_exams_v1(scrubbed_params) set_etag(exams[:version]) unless bypass_versioning? diff --git a/app/controllers/v1/gids/lcpe/lacs_controller.rb b/app/controllers/v1/gids/lcpe/lacs_controller.rb index fbc7ced8769..efa80c0b594 100644 --- a/app/controllers/v1/gids/lcpe/lacs_controller.rb +++ b/app/controllers/v1/gids/lcpe/lacs_controller.rb @@ -4,8 +4,6 @@ module V1 module GIDS module LCPE class LacsController < GIDS::LCPEController - VERSIONING_PARAMS = %i[version id].freeze - def index lacs = service.get_licenses_and_certs_v1(scrubbed_params) set_etag(lacs[:version]) unless bypass_versioning? diff --git a/app/controllers/v1/gids/lcpe_controller.rb b/app/controllers/v1/gids/lcpe_controller.rb index 86706a20ec2..09af2b1f3e1 100644 --- a/app/controllers/v1/gids/lcpe_controller.rb +++ b/app/controllers/v1/gids/lcpe_controller.rb @@ -16,11 +16,16 @@ def service end def lcpe_client - GI::LCPE::Client.new(version_id:, lcpe_type: controller_name) + GI::LCPE::Client.new(v_client: preload_version_id, lcpe_type: controller_name) end - def version_id - (request.headers['If-None-Match'] || params[:version]).to_s + def preload_version_id + preload_version_from_enriched_id || request.headers['If-None-Match']&.to_s + end + + # '@' + def preload_version_from_enriched_id + params[:id]&.split('@')&.last end def set_etag(version) @@ -29,11 +34,7 @@ def set_etag(version) # If additional filter params present, bypass versioning def bypass_versioning? - scrubbed_params.except(*versioning_params).present? - end - - def versioning_params - self.class::VERSIONING_PARAMS + scrubbed_params.except(:id).present? end def version_invalid diff --git a/lib/gi/configuration.rb b/lib/gi/configuration.rb index 790fb2a1735..d00889463a4 100644 --- a/lib/gi/configuration.rb +++ b/lib/gi/configuration.rb @@ -43,7 +43,7 @@ def request_headers end def versioning_enabled? - respond_to?(:etag) && etag.present? + try(:etag).present? end end end diff --git a/lib/gi/lcpe/client.rb b/lib/gi/lcpe/client.rb index 6cf57a6ca19..5d59e715c0b 100644 --- a/lib/gi/lcpe/client.rb +++ b/lib/gi/lcpe/client.rb @@ -11,8 +11,8 @@ class Client < GI::Client attr_accessor :redis_key, :v_client - def initialize(version_id: nil, lcpe_type: nil) - @v_client = version_id + def initialize(v_client: nil, lcpe_type: nil) + @v_client = v_client @redis_key = lcpe_type super() end @@ -26,7 +26,7 @@ def get_licenses_and_certs_v1(params = {}) def get_license_and_cert_details_v1(params = {}) validate_client_version do lac_id = params[:id] - perform(:get, "v1/lcpe/lacs/#{lac_id}", params.except(:id, :version)) + perform(:get, "v1/lcpe/lacs/#{lac_id}", params.except(:id)) end end @@ -37,13 +37,15 @@ def get_exams_v1(params = {}) end def get_exam_details_v1(params = {}) - exam_id = params[:id] - response = perform(:get, "v1/lcpe/exams/#{exam_id}", params.except(:id)) - gids_response(response) + validate_client_version do + exam_id = params[:id] + perform(:get, "v1/lcpe/exams/#{exam_id}", params.except(:id)) + end end private + # LCPE::Client can be called from GIDSRedis, in which case versioning is disabled def versioning_enabled? redis_key.present? end @@ -63,21 +65,20 @@ def v_cache @v_cache ||= lcpe_cache.cached_version end - # If versioning enabled, validate client has fresh collection before querying details + # Validate client has fresh collection before querying details def validate_client_version - return gids_response(yield) unless versioning_enabled? - # client (and not vets-api cache) must have fresh version config.etag = v_client - res = perform(:get, 'v1/lcpe/lacs', {}) - case res.status + validation_response = perform(:get, 'v1/lcpe/lacs', {}) + case validation_response.status when 304 config.etag = nil # version is fresh, redirect to query details - gids_response(yield).body + details_response = yield + gids_response(details_response).body else # version stale, client must refresh preloaded collection - lcpe_cache.force_client_refresh_and_cache(res) + lcpe_cache.force_client_refresh_and_cache(validation_response) end end diff --git a/spec/lib/gi/lcpe/client_spec.rb b/spec/lib/gi/lcpe/client_spec.rb index 13b42e2246d..f86110ce0f2 100644 --- a/spec/lib/gi/lcpe/client_spec.rb +++ b/spec/lib/gi/lcpe/client_spec.rb @@ -5,13 +5,13 @@ require 'gi/lcpe/response' describe GI::LCPE::Client do - let(:client) { described_class.new(version_id:, lcpe_type:) } + let(:client) { described_class.new(v_client:, lcpe_type:) } let(:v_fresh) { '3' } let(:v_stale) { '2' } describe '#get_licenses_and_certs_v1' do context 'when versioning disabled' do - let(:version_id) { nil } + let(:v_client) { nil } let(:lcpe_type) { nil } it 'defaults to GI::GIDSResponse' do @@ -24,7 +24,7 @@ end context 'when versioning enabled' do - let(:version_id) { v_fresh } + let(:v_client) { v_fresh } let(:lcpe_type) { 'lacs' } it 'defaults to GI::LCPE::Response' do @@ -46,7 +46,7 @@ end context 'when versioning disabled' do - let(:version_id) { nil } + let(:v_client) { nil } let(:lcpe_type) { nil } it 'creates GI::GIDSReponse to be passed to GIDSRedis' do @@ -62,23 +62,23 @@ let(:lcpe_type) { 'lacs' } context 'when client version stale' do - let(:version_id) { v_stale } + let(:v_client) { v_stale } it 'raises ClientCacheStaleError' do VCR.use_cassette('gi/lcpe/get_lacs_cache_stale') do - expect { client.get_license_and_cert_details_v1({ id: '1@f9822', version: version_id }) } + expect { client.get_license_and_cert_details_v1({ id: '1@f9822', version: v_client }) } .to raise_error(LCPERedis::ClientCacheStaleError) end end end context 'when client version fresh' do - let(:version_id) { v_fresh } + let(:v_client) { v_fresh } it 'creates GI::GIDSReponse and calls body' do VCR.use_cassette('gi/lcpe/get_lacs_cache_fresh') do VCR.use_cassette('gi/lcpe/get_lac_details') do - client.get_license_and_cert_details_v1({ id: '1@f9822', version: version_id }) + client.get_license_and_cert_details_v1({ id: '1@f9822', version: v_client }) expect(GI::GIDSResponse).to have_received(:new) expect(response).to have_received(:body) end @@ -90,7 +90,7 @@ describe '#get_exams_v1' do context 'when versioning disabled' do - let(:version_id) { nil } + let(:v_client) { nil } let(:lcpe_type) { nil } it 'defaults to GI::GIDSResponse' do @@ -103,7 +103,7 @@ end context 'when versioning enabled' do - let(:version_id) { v_fresh } + let(:v_client) { v_fresh } let(:lcpe_type) { 'exams' } it 'defaults to GI::LCPE::Response' do @@ -118,7 +118,7 @@ describe '#get_exam_details_v1' do let(:response) { instance_double(GI::GIDSResponse) } - let(:version_id) { nil } + let(:v_client) { nil } let(:lcpe_type) { nil } it 'creates GI::GIDSReponse to be passed to GIDSRedis' do