Simplify export unicity check and use create_or_find_by

app/models/export.rb

@@ -4,6 +4,7 @@
 #
 #  id         :bigint           not null, primary key
 #  format     :string           not null
+#  key        :text
 #  created_at :datetime         not null
 #  updated_at :datetime         not null
 #
@@ -20,9 +21,9 @@ class Export < ApplicationRecord
 
   has_one_attached :file
 
-  validates :format, :groupe_instructeurs, presence: true
+  validates :format, :groupe_instructeurs, :key, presence: true
 
-  scope :stale, -> { where('updated_at < ?', (Time.zone.now - MAX_DUREE_CONSERVATION_EXPORT)) }
+  scope :stale, -> { where('exports.updated_at < ?', (Time.zone.now - MAX_DUREE_CONSERVATION_EXPORT)) }
 
   after_create_commit :compute_async
 
@@ -45,35 +46,22 @@ class Export < ApplicationRecord
   end
 
   def self.find_or_create_export(format, groupe_instructeurs)
-    export = Export.find_for_format_and_groupe_instructeurs(format, groupe_instructeurs)
-
-    if export.nil?
-      export = Export.create(
-        format: format,
-        groupe_instructeurs: groupe_instructeurs
-      )
-    end
-
-    export
+    create_with(groupe_instructeurs: groupe_instructeurs)
+      .create_or_find_by(format: format, key: generate_cache_key(groupe_instructeurs))
   end
 
   def self.find_for_format_and_groupe_instructeurs(format, groupe_instructeurs)
-    export_including_gis = Export
-      .joins(:exports_groupe_instructeurs)
-      .where(
-        format: format,
-        exports_groupe_instructeurs: { groupe_instructeur: groupe_instructeurs }
-      )
+    find_by(format: format, key: generate_cache_key(groupe_instructeurs))
+  end
 
-    export_including_gis.find do |export|
-      export.groupe_instructeurs.pluck(:id).sort == groupe_instructeurs.map(&:id).sort
-    end
+  def self.generate_cache_key(groupe_instructeurs)
+    groupe_instructeurs.map(&:id).sort.join('-')
   end
 
   private
 
   def filename
-    procedure_identifier = procedure.path || "procedure-#{id}"
+    procedure_identifier = procedure.path || "procedure-#{procedure.id}"
     "dossiers_#{procedure_identifier}_#{Time.zone.now.strftime('%Y-%m-%d_%H-%M')}.#{format}"
   end
 

db/migrate/20210331123709_add_cache_key_to_exports.rb

@@ -0,0 +1,6 @@
+class AddCacheKeyToExports < ActiveRecord::Migration[6.1]
+  def change
+    add_column :exports, :key, :text
+    add_index :exports, [:format, :key], unique: true
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2021_03_30_112235) do
+ActiveRecord::Schema.define(version: 2021_03_31_123709) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -354,6 +354,8 @@ ActiveRecord::Schema.define(version: 2021_03_30_112235) do
     t.string "format", null: false
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.text "key"
+    t.index ["format", "key"], name: "index_exports_on_format_and_key", unique: true
   end
 
   create_table "exports_groupe_instructeurs", force: :cascade do |t|

spec/controllers/instructeurs/procedures_controller_spec.rb

@@ -441,7 +441,7 @@ describe Instructeurs::ProceduresController, type: :controller do
 
     context 'when the export is not ready' do
       before do
-        Export.create(format: :csv, groupe_instructeurs: [gi_1])
+        create(:export, groupe_instructeurs: [gi_1])
       end
 
       it 'displays an notice' do
@@ -451,9 +451,7 @@ describe Instructeurs::ProceduresController, type: :controller do
     end
 
     context 'when the export is ready' do
-      let!(:export) do
-        Export.create(format: :csv, groupe_instructeurs: [gi_1])
-      end
+      let!(:export) { create(:export, groupe_instructeurs: [gi_1]) }
 
       before do
         export.file.attach(io: StringIO.new('export'), filename: 'file.csv')
@@ -466,9 +464,7 @@ describe Instructeurs::ProceduresController, type: :controller do
     end
 
     context 'when another export is ready' do
-      let!(:export) do
-        Export.create(format: :csv, groupe_instructeurs: [gi_0, gi_1])
-      end
+      let!(:export) { create(:export, groupe_instructeurs: [gi_0, gi_1]) }
 
       before do
         export.file.attach(io: StringIO.new('export'), filename: 'file.csv')

spec/factories/export.rb

@@ -2,5 +2,9 @@ FactoryBot.define do
   factory :export do
     format { :csv }
     groupe_instructeurs { [association(:groupe_instructeur)] }
+
+    after(:build) do |export, _evaluator|
+      export.key = Export.generate_cache_key(export.groupe_instructeurs)
+    end
   end
 end

spec/models/export_spec.rb

@@ -3,7 +3,7 @@ RSpec.describe Export, type: :model do
     let(:groupe_instructeur) { create(:groupe_instructeur) }
 
     context 'when everything is ok' do
-      let(:export) { build(:export) }
+      let(:export) { build(:export, groupe_instructeurs: [groupe_instructeur]) }
 
       it { expect(export.save).to be true }
     end
@@ -15,7 +15,7 @@ RSpec.describe Export, type: :model do
     end
 
     context 'when format is missing' do
-      let(:export) { build(:export, format: nil) }
+      let(:export) { build(:export, format: nil, groupe_instructeurs: [groupe_instructeur]) }
 
       it { expect(export.save).to be false }
     end
@@ -46,7 +46,7 @@ RSpec.describe Export, type: :model do
     let!(:gi_3) { create(:groupe_instructeur, procedure: procedure) }
 
     context 'when an export is made for one groupe instructeur' do
-      let!(:export) { Export.create(format: :csv, groupe_instructeurs: [gi_1, gi_2]) }
+      let!(:export) { create(:export, groupe_instructeurs: [gi_1, gi_2]) }
 
       it { expect(Export.find_for_format_and_groupe_instructeurs(:csv, [gi_1])).to eq(nil) }
       it { expect(Export.find_for_format_and_groupe_instructeurs(:csv, [gi_2, gi_1])).to eq(export) }