refactor

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?

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?

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
+
+      # '<record id>@<preload version>'
+      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

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

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
 

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