From 4d1988fd66704afb80585febdee756e547cba56e Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Wed, 24 Jan 2024 09:44:24 +0100 Subject: [PATCH 1/2] chore(bundle): +kredis --- Gemfile | 1 + Gemfile.lock | 5 +++++ config/initializers/kredis.rb | 7 +++++++ 3 files changed, 13 insertions(+) create mode 100644 config/initializers/kredis.rb diff --git a/Gemfile b/Gemfile index 563fb5300..96aa00175 100644 --- a/Gemfile +++ b/Gemfile @@ -54,6 +54,7 @@ gem 'invisible_captcha' gem 'json_schemer' gem 'jwt' gem 'kaminari' +gem 'kredis' gem 'listen' # Required by ActiveSupport::EventedFileUpdateChecker gem 'lograge' gem 'logstash-event' diff --git a/Gemfile.lock b/Gemfile.lock index 236429576..cce772105 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -379,6 +379,10 @@ GEM activerecord kaminari-core (= 1.2.2) kaminari-core (1.2.2) + kredis (1.7.0) + activemodel (>= 6.0.0) + activesupport (>= 6.0.0) + redis (>= 4.2, < 6) launchy (2.5.0) addressable (~> 2.7) letter_opener (1.7.0) @@ -876,6 +880,7 @@ DEPENDENCIES json_schemer jwt kaminari + kredis launchy letter_opener_web listen diff --git a/config/initializers/kredis.rb b/config/initializers/kredis.rb new file mode 100644 index 000000000..878b8177a --- /dev/null +++ b/config/initializers/kredis.rb @@ -0,0 +1,7 @@ +redis_volatile_options = { + url: ENV['REDIS_CACHE_URL'], # will fallback to default redis url if empty, and won't fail if there is no redis server + ssl: ENV['REDIS_CACHE_SSL'] == 'enabled' +} +redis_volatile_options[:ssl_params] = { verify_mode: OpenSSL::SSL::VERIFY_NONE } if ENV['REDIS_CACHE_SSL_VERIFY_NONE'] == 'enabled' + +Kredis::Connections.connections[:volatile] = Redis.new(redis_volatile_options) From 56f8b61866b29c41396f0d691b813701a5505b2d Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Wed, 24 Jan 2024 10:04:31 +0100 Subject: [PATCH 2/2] 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