APPEALS-24400 added sync lock concern and error

app/jobs/decision_issue_sync_job.rb

@@ -11,6 +11,10 @@ def perform(request_issue_or_effectuation)
 
     begin
       request_issue_or_effectuation.sync_decision_issues!
+    rescue Caseflow::Error::SyncLockFailed => error
+      request_issue_or_effectuation.update_error!(error.inspect)
+      request_issue_or_effectuation.update!(decision_sync_attempted_at: Time.zone.now - 11.hours - 55.minutes)
+      capture_exception(error: error)
     rescue Errno::ETIMEDOUT => error
       # no Raven report. We'll try again later.
       Rails.logger.error error

app/models/concerns/sync_lock.rb

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+require "redis"
+
+module SyncLock
+  extend ActiveSupport::Concern
+  LOCK_TIMEOUT = ENV["SYNC_LOCK_MAX_DURATION"]
+
+  def hlr_sync_lock
+    if decision_review.is_a?(HigherLevelReview) && block_given?
+      redis = Redis.new(url: Rails.application.secrets.redis_url_cache)
+      lock_key = "hlr_sync_lock:#{end_product_establishment.id}"
+
+      begin
+        sync_lock_acquired = redis.setnx(lock_key, true)
+
+        fail Caseflow::Error::SyncLockFailed, message: "#{Time.zone.now}" unless sync_lock_acquired
+
+        redis.expire(lock_key, LOCK_TIMEOUT.to_i)
+        yield
+      ensure
+        redis.del(lock_key)
+      end
+    elsif block_given?
+      yield
+    end
+  end
+end

app/models/request_issue.rb

@@ -11,6 +11,7 @@ class RequestIssue < CaseflowRecord
   include HasBusinessLine
   include DecisionSyncable
   include HasDecisionReviewUpdatedSince
+  include SyncLock
 
   # how many days before we give up trying to sync decisions
   REQUIRES_PROCESSING_WINDOW_DAYS = 30
@@ -431,14 +432,23 @@ def sync_decision_issues!
     # to avoid a slow BGS call causing the transaction to timeout
     end_product_establishment.veteran
 
-    transaction do
-      return unless create_decision_issues
-
-      end_product_establishment.on_decision_issue_sync_processed(self)
-      clear_error!
-      close_decided_issue!
-      processed!
+    ### hlr_sync_lock will stop any other request issues associated with the current End Product Establishment
+    ### from syncing with BGS concurrently if the claim is a Higher Level Review. This will ensure that
+    ### the remand supplemental claim generation that occurs within '#on_decision_issue_sync_processed' will
+    ### not be inadvertantly bypassed due to two request issues from the same claim being synced at the same
+    ### time. If this situation does occur, one of the request issues will error out with
+    ### Caseflow::Error:SyncLockFailed and be picked up to sync again later
+    hlr_sync_lock do
+      transaction do
+        return unless create_decision_issues
+
+        end_product_establishment.on_decision_issue_sync_processed(self)
+        clear_error!
+        close_decided_issue!
+        processed!
+      end
     end
+
   end
 
   def vacols_issue

config/environments/development.rb

@@ -93,6 +93,9 @@
   # Travel Board Sync Batch Size
   ENV["TRAVEL_BOARD_HEARING_SYNC_BATCH_LIMIT"] ||= "250"
 
+  # Time in seconds before the sync lock expires
+  LOCK_TIMEOUT = ENV["SYNC_LOCK_MAX_DURATION"] ||= "60"
+
   # Raises error for missing translations
   # config.action_view.raise_on_missing_translations = true
   # Notifications page eFolder link

config/environments/test.rb

@@ -108,6 +108,9 @@
   # Travel Board Sync Batch Size
   ENV["TRAVEL_BOARD_HEARING_SYNC_BATCH_LIMIT"] ||= "250"
 
+  # Time in seconds before the sync lock expires
+  LOCK_TIMEOUT = ENV["SYNC_LOCK_MAX_DURATION"] ||= "60"
+
   # Notifications page eFolder link
   ENV["CLAIM_EVIDENCE_EFOLDER_BASE_URL"] ||= "https://vefs-claimevidence-ui-uat.stage.bip.va.gov"
 

lib/caseflow/error.rb

@@ -458,4 +458,10 @@ class PacmanBadRequestError < PacmanApiError; end
   class PacmanForbiddenError < PacmanApiError; end
   class PacmanNotFoundError < PacmanApiError; end
   class PacmanInternalServerError < PacmanApiError; end
+
+  class SyncLockFailed < StandardError
+    def ignorable?
+      true
+    end
+  end
 end

spec/jobs/decision_issue_sync_job_spec.rb

@@ -5,11 +5,13 @@
   let(:request_issue) { create(:request_issue, end_product_establishment: epe) }
   let(:no_ratings_err) { Rating::NilRatingProfileListError.new("none!") }
   let(:bgs_transport_err) { BGS::ShareError.new("network!") }
+  let(:sync_lock_err) {Caseflow::Error::SyncLockFailed.new("#{Time.zone.now}")}
 
   subject { described_class.perform_now(request_issue) }
 
   before do
     @raven_called = false
+    Timecop.freeze(Time.utc(2023, 1, 1, 12, 0, 0))
   end
 
   it "ignores NilRatingProfileListError for Sentry, logs on db" do
@@ -42,6 +44,16 @@
     expect(@raven_called).to eq(true)
   end
 
+  it "logs SyncLock errors" do
+    capture_raven_log
+    allow(request_issue).to receive(:sync_decision_issues!).and_raise(sync_lock_err)
+
+    subject
+    expect(request_issue.decision_sync_error).to eq("#<Caseflow::Error::SyncLockFailed: #{Time.zone.now}>")
+    expect(request_issue.decision_sync_attempted_at).to be_within(5.minutes).of 12.hours.ago
+    expect(@raven_called).to eq(false)
+  end
+
   it "ignores error on success" do
     allow(request_issue).to receive(:sync_decision_issues!).and_return(true)
 

spec/models/request_issue_spec.rb

@@ -2111,6 +2111,124 @@
             expect(nonrating_request_issue.processed?).to eq(false)
           end
         end
+
+        context "when hlr_sync_lock is applied to the sync method" do
+          let(:ep_code) { "030HLRR" }
+          let!(:epe) do
+            epe = create(
+              :end_product_establishment,
+              :cleared,
+              established_at: 5.days.ago,
+              modifier: "030",
+              code: "030HLRR",
+              source: create(
+                :higher_level_review,
+                veteran_file_number: veteran.file_number
+              )
+            )
+            EndProductEstablishment.find epe.id
+          end
+          let!(:review) do
+            epe.source
+          end
+
+          let!(:contention_hlr1) do
+            Generators::Contention.build(
+              id: "123456789",
+              claim_id: epe.reference_id,
+              disposition: "Difference of Opinion"
+            )
+          end
+          let!(:contention_hlr2) do
+            Generators::Contention.build(
+              id: "555566660",
+              claim_id: epe.reference_id,
+              disposition: "DTA Error"
+            )
+          end
+
+          let(:original_decision_sync_last_submitted_at) { Time.zone.now - 1.hour }
+          let(:original_decision_sync_submitted_at) { Time.zone.now - 1.hour }
+
+          let(:request_issue1) do
+            create(
+              :request_issue,
+              decision_review: review,
+              nonrating_issue_description: "some description",
+              nonrating_issue_category: "a category",
+              decision_date: 1.day.ago,
+              end_product_establishment: epe,
+              contention_reference_id: contention_hlr1.id,
+              benefit_type: review.benefit_type,
+              decision_sync_last_submitted_at: original_decision_sync_last_submitted_at,
+              decision_sync_submitted_at: original_decision_sync_submitted_at
+            )
+          end
+
+          let(:request_issue2) do
+            create(
+              :request_issue,
+              decision_review: review,
+              nonrating_issue_description: "some description",
+              nonrating_issue_category: "a category",
+              decision_date: 1.day.ago,
+              end_product_establishment: epe,
+              contention_reference_id: contention_hlr2.id,
+              benefit_type: review.benefit_type,
+              decision_sync_last_submitted_at: original_decision_sync_last_submitted_at,
+              decision_sync_submitted_at: original_decision_sync_submitted_at
+            )
+          end
+
+          let!(:claimant) do
+            Claimant.create!(decision_review: epe.source,
+                             participant_id: epe.veteran.participant_id,
+                             payee_code: "00")
+          end
+
+          let(:sync_lock_err) { Caseflow::Error::SyncLockFailed }
+
+          it "prevents a request issue from acquiring the SyncLock when there is already a lock using the EPE's ID" do
+            redis = Redis.new(url: Rails.application.secrets.redis_url_cache)
+            lock_key = "hlr_sync_lock:#{epe.id}"
+            redis.setnx(lock_key, true)
+            expect { request_issue1.sync_decision_issues! }.to raise_error(sync_lock_err)
+            redis.del(lock_key)
+          end
+
+          it "allows a request issue to sync if there is no existing lock using the EPE's ID" do
+            expect(request_issue2.sync_decision_issues!).to eq(true)
+            expect(request_issue2.processed?).to eq(true)
+
+            expect(SupplementalClaim.count).to eq(1)
+          end
+
+          it "multiple request issues can sync and a remand_supplemental_claim is created" do
+            expect(request_issue1.sync_decision_issues!).to eq(true)
+            expect(request_issue2.sync_decision_issues!).to eq(true)
+            expect(request_issue1.processed?).to eq(true)
+            expect(request_issue2.processed?).to eq(true)
+
+            expect(SupplementalClaim.count).to eq(1)
+            sc = SupplementalClaim.first
+            expect(sc.request_issues.count).to eq(2)
+            supplemental_claim_request_issue1 = sc.request_issues.first
+            supplemental_claim_request_issue2= sc.request_issues.last
+
+            # both request issues link to the same SupplementalClaim
+            expect(sc.id).to eq (request_issue1.end_product_establishment.source.remand_supplemental_claims.first.id)
+            expect(sc.id).to eq (request_issue2.end_product_establishment.source.remand_supplemental_claims.first.id)
+
+            # DecisionIssue ID should match contested_decision_issue_id
+            expect(DecisionIssue.count).to eq(2)
+            decision_issue1 = DecisionIssue.first
+            decision_issue2 = DecisionIssue.last
+
+            expect(decision_issue1.id).to eq(supplemental_claim_request_issue1.contested_decision_issue_id)
+            expect(decision_issue2.id).to eq(supplemental_claim_request_issue2.contested_decision_issue_id)
+          end
+
+        end
       end
     end
   end