From 56f8b61866b29c41396f0d691b813701a5505b2d Mon Sep 17 00:00:00 2001
From: Colin Darie <colin@darie.eu>
Date: Wed, 24 Jan 2024 10:04:31 +0100
Subject: [PATCH] feat(dossiers): lock submit en construction to avoid merge
 collisions with double submits

---
 .github/workflows/ci.yml                      |  2 +-
 app/controllers/concerns/lockable_concern.rb  | 16 +++++++
 app/controllers/users/dossiers_controller.rb  |  5 ++
 .../concerns/lockable_concern_spec.rb         | 46 +++++++++++++++++++
 4 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 app/controllers/concerns/lockable_concern.rb
 create mode 100644 spec/controllers/concerns/lockable_concern_spec.rb

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index fd19fd521..8912b6e49 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -77,7 +77,7 @@ jobs:
       - name: Install build dependancies
         # - fonts pickable by ImageMagick
         # - rust for YJIT support
-        run: sudo apt-get install -y gsfonts rustc
+        run: sudo apt-get install -y gsfonts rustc redis-server
 
       - name: Setup the app runtime and dependencies
         uses: ./.github/actions/ci-setup-rails
diff --git a/app/controllers/concerns/lockable_concern.rb b/app/controllers/concerns/lockable_concern.rb
new file mode 100644
index 000000000..f6e59743b
--- /dev/null
+++ b/app/controllers/concerns/lockable_concern.rb
@@ -0,0 +1,16 @@
+module LockableConcern
+  extend ActiveSupport::Concern
+
+  included do
+    def lock_action(key)
+      lock = Kredis.flag(key, config: :volatile)
+      head :locked and return if lock.marked?
+
+      lock.mark(expires_in: 10.seconds)
+
+      yield
+
+      lock.remove
+    end
+  end
+end
diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb
index 88e1930a3..4a3c53477 100644
--- a/app/controllers/users/dossiers_controller.rb
+++ b/app/controllers/users/dossiers_controller.rb
@@ -2,6 +2,7 @@ module Users
   class DossiersController < UserController
     include DossierHelper
     include TurboChampsConcern
+    include LockableConcern
 
     layout 'procedure_context', only: [:identite, :update_identite, :siret, :update_siret]
 
@@ -19,6 +20,10 @@ module Users
     before_action :show_demarche_en_test_banner
     before_action :store_user_location!, only: :new
 
+    around_action only: :submit_en_construction do |_controller, action|
+      lock_action("lock-submit-en-construction-#{@dossier.id}", &action)
+    end
+
     def index
       ordered_dossiers = Dossier.includes(:procedure).order_by_updated_at
       deleted_dossiers = current_user.deleted_dossiers.includes(:procedure).order_by_updated_at
diff --git a/spec/controllers/concerns/lockable_concern_spec.rb b/spec/controllers/concerns/lockable_concern_spec.rb
new file mode 100644
index 000000000..add57dc8d
--- /dev/null
+++ b/spec/controllers/concerns/lockable_concern_spec.rb
@@ -0,0 +1,46 @@
+describe LockableConcern, type: :controller do
+  controller(ApplicationController) do
+    include LockableConcern
+
+    def test_action
+      lock_action(params.fetch(:lock_key)) do
+        render plain: 'Action completed'
+      end
+    end
+  end
+
+  before do
+    routes.draw { get 'test_action/:lock_key' => 'anonymous#test_action' }
+  end
+
+  describe '#lock_action' do
+    # randomize key to avoid collision on concurrent tests
+    let(:lock_key) { "test_lock_#{SecureRandom.uuid}" }
+    subject { get :test_action, params: { lock_key: } }
+
+    context 'when there is no concurrent request' do
+      it 'completes the action' do
+        expect(subject).to have_http_status(:ok)
+      end
+    end
+
+    context 'when there are concurrent requests' do
+      it 'aborts the second request' do
+        # Simulating the first request acquiring the lock
+        Kredis.flag(lock_key, config: :volatile).mark(expires_in: 3.seconds)
+
+        # Making the second request
+        expect(subject).to have_http_status(:locked)
+      end
+    end
+
+    context 'when the lock expires' do
+      it 'allows another request after expiration' do
+        Kredis.flag(lock_key, config: :volatile).mark(expires_in: 0.001.seconds)
+        sleep 0.002
+
+        expect(subject).to have_http_status(:ok)
+      end
+    end
+  end
+end