Merge pull request #3928 from sanger/dpl957_fix_local_integration_suite

Dpl957 fix local integration suite

app/models/plate_barcode.rb

@@ -103,11 +103,20 @@ def self._connection_scope(url, data = nil)
   if Rails.env.development?
     # If we don't want a test dependency on baracoda we need to mock barcodes and child barcodes
 
+    #
+    # When in development we were receiving concurrent requests for a barcode
+    # we were experiencing a race condition where both requests were obtaining the same barcode.
+    # To avoid it we will cache the barcodes and perform retry until unique barcode is
+    # obtained
+    extend Dev::PlateBarcode::CacheBarcodes
+
     def self.create_barcode
       # We should use a different prefix for local so that you can switch between using baracoda
       # locally and there will not be clashes
-      current_num = Barcode.sequencescape22.order(id: :desc).first&.number || 9000
-      Barcode.build_sequencescape22({ barcode: "#{prefix}-#{current_num + 1}" })
+
+      # We cache the last barcode so we don't get race conditions on generating barcodes
+      # when requests appear at the same time
+      Barcode.build_sequencescape22({ barcode: "#{prefix}-#{dev_cache_get_next_barcode}" })
     end
 
     def self.create_child_barcodes(parent_barcode, count = 1)

config/default_records/plate_purposes/005_limber_purposes.yml

@@ -47,7 +47,7 @@ LHR-384 RT:
 # Later, included as an alternative entry point to the Limber Bespoke PCR pipeline, and made
 # a cherrypickable_target to add flexibility.
 LBC Cherrypick:
-  type: PlatePurpose::Input
+  type: PlatePurpose
   cherrypickable_target: true
 # Towards the end of the WGS process, forms the attachment point for the multiplexing
 # requests

lib/dev/plate_barcode/cache_barcodes.rb

@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+module Dev
+  module PlateBarcode
+    # When in development mode we were receiving concurrent requests for creating a barcode
+    # which produced a race condition where all concurrent requests were obtaining the same barcode.
+    # To avoid it this module implements a cache of barcodes so it should be able to support
+    # up to MAX_SIZE_CACHE concurrent requests which we expect should be enough for
+    # development.
+    module CacheBarcodes
+      # Obtains a new barcode and if the obtained barcode was obtained before (if it was cached)
+      # it will retry and obtain a new barcode
+      def dev_cache_get_next_barcode
+        pos = 1
+        next_barcode = nil
+        loop do
+          next_barcode = (Barcode.sequencescape22.order(id: :desc).first&.number || 9000) + pos
+          pos += 1
+          break unless barcode_in_cache?(next_barcode)
+        end
+        cache_barcode(next_barcode)
+        next_barcode
+      end
+
+      MAX_SIZE_CACHE = 100
+
+      # Cache stored in a class property (in PlateBarcode)
+      def data_cache
+        @data_cache ||= []
+      end
+
+      def reset_cache
+        @data_cache = []
+      end
+
+      # Avoids the cache to grow out of the cache limits
+      def resize_cache
+        @data_cache = data_cache.drop(1) if data_cache.length >= MAX_SIZE_CACHE
+      end
+
+      def cache_barcode(barcode)
+        resize_cache
+
+        data_cache.push(barcode)
+      end
+
+      def barcode_in_cache?(barcode)
+        data_cache.include?(barcode)
+      end
+    end
+  end
+end

spec/lib/dev/plate_barcode/cache_barcodes_spec.rb

@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+require 'rails_helper'
+
+RSpec.describe Dev::PlateBarcode::CacheBarcodes do
+  describe '#dev_cache_get_next_barcode' do
+    let(:my_class) { Class.new { extend Dev::PlateBarcode::CacheBarcodes } }
+    let(:instance) { my_class.new }
+
+    context 'when getting new barcodes' do
+      before { my_class.reset_cache }
+
+      it 'gets different barcodes on each call' do
+        expect(my_class.dev_cache_get_next_barcode).to eq(9001)
+        expect(my_class.dev_cache_get_next_barcode).to eq(9002)
+        expect(my_class.dev_cache_get_next_barcode).to eq(9003)
+      end
+    end
+
+    context 'with the cache management' do
+      before { my_class.reset_cache }
+
+      it 'cache grows with every new barcode' do
+        expect { my_class.dev_cache_get_next_barcode }.to change { my_class.data_cache.length }.by(1)
+        expect do
+          my_class.dev_cache_get_next_barcode
+          my_class.dev_cache_get_next_barcode
+          my_class.dev_cache_get_next_barcode
+        end.to change { my_class.data_cache.length }.by(3)
+      end
+
+      it 'does not grow over the max size' do
+        pos = 0
+        while pos < Dev::PlateBarcode::CacheBarcodes::MAX_SIZE_CACHE
+          my_class.dev_cache_get_next_barcode
+          pos += 1
+        end
+
+        expect do
+          my_class.dev_cache_get_next_barcode
+          my_class.dev_cache_get_next_barcode
+          my_class.dev_cache_get_next_barcode
+        end.not_to(change { my_class.data_cache.length })
+      end
+    end
+  end
+end