Prevent race during school creation per creator

lib/concepts/school/operations/create.rb

@@ -2,12 +2,19 @@
 
 class School
   class Create
+    LOCK_NAMESPACE = Zlib.crc32(name)
+
     class << self
       def call(school_params:, creator_id:, token:)
         response = OperationResponse.new
         response[:school] = build_school(school_params.merge!(creator_id:))
 
         School.transaction do
+          # Serialise concurrent creates by the same creator.
+          # The loser blocks here until the winner's transaction commits,
+          # so its uniqueness validation sees the winner's school
+          # rather than hitting the partial unique index on creator_id.
+          acquire_advisory_lock_for_creator(creator_id)
           response[:school].save!
 
           SchoolOnboardingService.new(response[:school]).onboard(token:)
@@ -17,7 +24,12 @@ def call(school_params:, creator_id:, token:)
       rescue ProfileApiClient::UnauthorizedError => e
         # Do not log noise to sentry.
         # TODO: consider returning a separate error here to distinguish from other errors and return 401 from the API, not 422
-        Rails.logger.warn { "Failed to onboard school #{response[:school].id}: user is unauthorized" }
+        Rails.logger.warn { "Failed to onboard school #{response[:school]&.id}: user is unauthorized" }
+        failure(response, e)
+      rescue ActiveRecord::RecordInvalid => e
+        # A double submit loses the advisory lock race and fails the creator_id
+        # uniqueness validation as expected, so keep it out of Sentry.
+        Sentry.capture_exception(e) unless response[:school]&.errors&.of_kind?(:creator_id, :taken)
         failure(response, e)
       rescue StandardError => e
         Sentry.capture_exception(e)
@@ -26,9 +38,15 @@ def call(school_params:, creator_id:, token:)
 
       private
 
+      def acquire_advisory_lock_for_creator(creator_id)
+        lock_key = Zlib.crc32("#{School::Create::LOCK_NAMESPACE}:#{creator_id}")
+        School.connection.execute("SELECT pg_advisory_xact_lock(#{lock_key})")
+      end
+
       def failure(response, error)
-        response[:error] = response[:school].errors.presence || [error.message]
-        response[:error_types] = response[:school].errors.details
+        school_errors = response[:school]&.errors
+        response[:error] = school_errors.presence || [error.message]
+        response[:error_types] = school_errors&.details || {}
         response
       end
 

spec/concepts/school/create_spec.rb

@@ -27,6 +27,13 @@
     allow(ProfileApiClient).to receive(:create_school).and_return(true)
   end
 
+  it 'acquires an advisory lock keyed on the creator' do
+    allow(School.connection).to receive(:execute).and_call_original
+    described_class.call(school_params:, creator_id:, token:)
+    lock_key = Zlib.crc32("#{School::Create::LOCK_NAMESPACE}:#{creator_id}")
+    expect(School.connection).to have_received(:execute).with("SELECT pg_advisory_xact_lock(#{lock_key})")
+  end
+
   it 'returns a successful operation response' do
     response = described_class.call(school_params:, creator_id:, token:)
     expect(response.success?).to be(true)
@@ -51,6 +58,41 @@
     expect(response[:school].creator_id).to eq(creator_id)
   end
 
+  context 'when the creator already has an active school' do
+    # The advisory lock serialises concurrent requests: the loser only validates
+    # once the winner has committed, so it sees the winner's school.
+    before do
+      allow(Sentry).to receive(:capture_exception)
+      create(:school, creator_id:, reference: '999999')
+    end
+
+    it 'does not create a second school' do
+      expect { described_class.call(school_params:, creator_id:, token:) }.not_to change(School, :count)
+    end
+
+    it 'returns a failed operation response' do
+      response = described_class.call(school_params:, creator_id:, token:)
+      expect(response.failure?).to be(true)
+    end
+
+    it 'returns a validation error rather than a database uniqueness violation' do
+      response = described_class.call(school_params:, creator_id:, token:)
+      expect(response[:error_types][:creator_id]).to eq([{ error: :taken, value: creator_id }])
+    end
+
+    it 'does not onboard the second school' do
+      allow(SchoolOnboardingService).to receive(:new)
+      described_class.call(school_params:, creator_id:, token:)
+      expect(SchoolOnboardingService).not_to have_received(:new)
+    end
+
+    it 'does not capture the error in Sentry' do
+      allow(Sentry).to receive(:capture_exception)
+      described_class.call(school_params:, creator_id:, token:)
+      expect(Sentry).not_to have_received(:capture_exception)
+    end
+  end
+
   context 'when creation fails' do
     let(:school_params) { {} }