Merge pull request #8079 from tchak/refactor-store-opeartions-as-jsonb

refactor(operation_log): store data in jsonb instead of files

app/jobs/cron/dossier_operation_log_move_to_cold_storage_job.rb

@@ -0,0 +1,9 @@
+class Cron::DossierOperationLogMoveToColdStorageJob < Cron::CronJob
+  self.schedule_expression = "every day at 1 am"
+
+  def perform
+    DossierOperationLog
+      .with_data
+      .find_each(&:move_to_cold_storage!)
+  end
+end

app/lib/active_storage/downloadable_file.rb

@@ -10,7 +10,7 @@ class ActiveStorage::DownloadableFile
     end
 
     files.filter do |file, _filename|
-      if file.is_a?(PiecesJustificativesService::FakeAttachment)
+      if file.is_a?(ActiveStorage::FakeAttachment)
         true
       else
         service = file.blob.service

app/lib/active_storage/fake_attachment.rb

@@ -0,0 +1,24 @@
+class ActiveStorage::FakeAttachment < Hashie::Dash
+  property :filename
+  property :name
+  property :file
+  property :id
+  property :created_at
+  property :record_type, default: 'Fake'
+
+  def download
+    file.read
+  end
+
+  def read(*args)
+    file.read(*args)
+  end
+
+  def close
+    file.close
+  end
+
+  def attached?
+    true
+  end
+end

app/lib/download_manager/parallel_download_queue.rb

@@ -32,7 +32,7 @@ module DownloadManager
       attachment_dir = File.dirname(attachment_path)
 
       FileUtils.mkdir_p(attachment_dir) if !Dir.exist?(attachment_dir) # defensive, do not write in undefined dir
-      if attachment.is_a?(PiecesJustificativesService::FakeAttachment)
+      if attachment.is_a?(ActiveStorage::FakeAttachment)
         File.write(attachment_path, attachment.file.read, mode: 'wb')
       else
         request = Typhoeus::Request.new(attachment.url)

app/models/dossier.rb

@@ -765,8 +765,8 @@ class Dossier < ApplicationRecord
   def expired_keep_track_and_destroy!
     transaction do
       DeletedDossier.create_from_dossier(self, :expired)
-      dossier_operation_logs.destroy_all
       log_automatic_dossier_operation(:supprimer, self)
+      dossier_operation_logs.purge_discarded
       destroy!
     end
     true
@@ -1164,7 +1164,7 @@ class Dossier < ApplicationRecord
   def purge_discarded
     transaction do
       DeletedDossier.create_from_dossier(self, hidden_by_reason)
-      dossier_operation_logs.not_deletion.destroy_all
+      dossier_operation_logs.purge_discarded
       destroy
     end
   end

app/models/dossier_operation_log.rb

@@ -4,6 +4,7 @@
 #
 #  id                  :bigint           not null, primary key
 #  automatic_operation :boolean          default(FALSE), not null
+#  data                :jsonb
 #  digest              :text
 #  executed_at         :datetime
 #  keep_until          :datetime
@@ -36,10 +37,29 @@ class DossierOperationLog < ApplicationRecord
   belongs_to :bill_signature, optional: true
 
   scope :not_deletion, -> { where.not(operation: operations.fetch(:supprimer)) }
+  scope :with_data, -> { where.not(data: nil) }
   scope :brouillon_expired, -> { where(dossier: Dossier.brouillon_expired).not_deletion }
   scope :en_construction_expired, -> { where(dossier: Dossier.en_construction_expired).not_deletion }
   scope :termine_expired, -> { where(dossier: Dossier.termine_expired).not_deletion }
 
+  def move_to_cold_storage!
+    if data.present?
+      serialized.attach(
+        io: StringIO.new(data.to_json),
+        filename: "operation-#{digest}.json",
+        content_type: 'application/json',
+        # we don't want to run virus scanner on this file
+        metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE }
+      )
+      update!(data: nil)
+    end
+  end
+
+  def self.purge_discarded
+    not_deletion.destroy_all
+    with_data.each(&:move_to_cold_storage!)
+  end
+
   def self.create_and_serialize(params)
     dossier = params.fetch(:dossier)
 
@@ -58,24 +78,17 @@ class DossierOperationLog < ApplicationRecord
       executed_at: Time.zone.now,
       automatic_operation: !!params[:automatic_operation])
 
-    serialized = {
+    data = {
       operation: operation_log.operation,
       dossier_id: operation_log.dossier_id,
       author: self.serialize_author(params[:author]),
       subject: self.serialize_subject(params[:subject], operation_log.operation),
       automatic_operation: operation_log.automatic_operation?,
       executed_at: operation_log.executed_at.iso8601
-    }.compact.to_json
+    }.compact
 
-    operation_log.digest = Digest::SHA256.hexdigest(serialized)
-
-    operation_log.serialized.attach(
-      io: StringIO.new(serialized),
-      filename: "operation-#{operation_log.digest}.json",
-      content_type: 'application/json',
-      # we don't want to run virus scanner on this file
-      metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE }
-    )
+    operation_log.data = data
+    operation_log.digest = Digest::SHA256.hexdigest(data.to_json)
 
     operation_log.save!
   end

app/services/pieces_justificatives_service.rb

@@ -77,34 +77,6 @@ class PiecesJustificativesService
     end
   end
 
-  class FakeAttachment < Hashie::Dash
-    property :filename
-    property :name
-    property :file
-    property :id
-    property :created_at
-
-    def download
-      file.read
-    end
-
-    def read(*args)
-      file.read(*args)
-    end
-
-    def close
-      file.close
-    end
-
-    def attached?
-      true
-    end
-
-    def record_type
-      'Fake'
-    end
-  end
-
   def self.generate_dossier_export(dossiers)
     return [] if dossiers.empty?
 
@@ -123,7 +95,7 @@ class PiecesJustificativesService
                   dossier: dossier
                 })
 
-      a = FakeAttachment.new(
+      a = ActiveStorage::FakeAttachment.new(
         file: StringIO.new(pdf),
         filename: "export-#{dossier.id}.pdf",
         name: 'pdf_export_for_instructeur',
@@ -229,14 +201,21 @@ class PiecesJustificativesService
 
   def self.operation_logs_and_signature_ids(dossiers)
     dol_id_dossier_id_bill_id = DossierOperationLog
+      .where(dossier: dossiers, data: nil)
+      .pluck(:bill_signature_id, :id, :dossier_id)
+    dol_id_data_bill_id = DossierOperationLog
       .where(dossier: dossiers)
-      .pluck(:id, :dossier_id, :bill_signature_id)
+      .with_data
+      .pluck(:bill_signature_id, :id, :dossier_id, :data, :digest, :created_at)
 
     dol_id_dossier_id = dol_id_dossier_id_bill_id
-      .map { |dol_id, dossier_id, _| [dol_id, dossier_id] }
+      .map { |_, dol_id, dossier_id| [dol_id, dossier_id] }
       .to_h
 
-    bill_ids = dol_id_dossier_id_bill_id.map(&:third).uniq.compact
+    bill_ids = (dol_id_dossier_id_bill_id + dol_id_data_bill_id)
+      .map(&:first)
+      .uniq
+      .compact
 
     serialized_dols = ActiveStorage::Attachment
       .includes(:blob)
@@ -245,6 +224,17 @@ class PiecesJustificativesService
         dossier_id = dol_id_dossier_id[a.record_id]
         ActiveStorage::DownloadableFile.pj_and_path(dossier_id, a)
       end
+    serialized_dols += dol_id_data_bill_id.map do |_, id, dossier_id, data, digest, created_at|
+      a = ActiveStorage::FakeAttachment.new(
+        file: StringIO.new(data.to_json),
+        filename: "operation-#{digest}.json",
+        name: 'serialized',
+        id: id,
+        created_at: created_at,
+        record_type: 'DossierOperationLog'
+      )
+      ActiveStorage::DownloadableFile.pj_and_path(dossier_id, a)
+    end
 
     [serialized_dols, bill_ids]
   end

db/migrate/20220315102928_add_data_to_dossier_operation_logs.rb

@@ -0,0 +1,5 @@
+class AddDataToDossierOperationLogs < ActiveRecord::Migration[6.1]
+  def change
+    add_column :dossier_operation_logs, :data, :jsonb
+  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: 2022_11_22_123809) do
+ActiveRecord::Schema.define(version: 2022_11_22_124009) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pgcrypto"
@@ -273,6 +273,7 @@ ActiveRecord::Schema.define(version: 2022_11_22_123809) do
     t.datetime "keep_until"
     t.string "operation", null: false
     t.datetime "updated_at", null: false
+    t.jsonb "data"
     t.index ["bill_signature_id"], name: "index_dossier_operation_logs_on_bill_signature_id"
     t.index ["dossier_id"], name: "index_dossier_operation_logs_on_dossier_id"
     t.index ["keep_until"], name: "index_dossier_operation_logs_on_keep_until"

spec/lib/download_manager/parallel_download_queue_spec.rb

@@ -13,7 +13,7 @@ describe DownloadManager::ParallelDownloadQueue do
 
     let(:destination) { 'lol.png' }
     let(:attachment) do
-      PiecesJustificativesService::FakeAttachment.new(
+      ActiveStorage::FakeAttachment.new(
         file: StringIO.new('coucou'),
         filename: "export-dossier.pdf",
         name: 'pdf_export_for_instructeur',
@@ -22,7 +22,7 @@ describe DownloadManager::ParallelDownloadQueue do
       )
     end
 
-    context 'with a PiecesJustificativesService::FakeAttachment and it works' do
+    context 'with a ActiveStorage::FakeAttachment and it works' do
       it 'write attachment.file to disk' do
         target = File.join(download_to_dir, destination)
         expect { subject }.to change { File.exist?(target) }
@@ -31,7 +31,7 @@ describe DownloadManager::ParallelDownloadQueue do
       end
     end
 
-    context 'with a PiecesJustificativesService::FakeAttachment and it fails' do
+    context 'with a ActiveStorage::FakeAttachment and it fails' do
       it 'write attachment.file to disk' do
         expect(attachment.file).to receive(:read).and_raise("boom")
         target = File.join(download_to_dir, destination)

spec/models/dossier_spec.rb

@@ -1043,7 +1043,7 @@ describe Dossier do
   describe '#accepter!' do
     let(:dossier) { create(:dossier, :en_instruction, :with_individual) }
     let(:last_operation) { dossier.dossier_operation_logs.last }
-    let(:operation_serialized) { JSON.parse(last_operation.serialized.download) }
+    let(:operation_serialized) { last_operation.data }
     let!(:instructeur) { create(:instructeur) }
     let!(:now) { Time.zone.parse('01/01/2100') }
     let(:attestation) { Attestation.new }
@@ -1105,7 +1105,7 @@ describe Dossier do
   describe '#passer_en_instruction!' do
     let(:dossier) { create(:dossier, :en_construction, en_construction_close_to_expiration_notice_sent_at: Time.zone.now) }
     let(:last_operation) { dossier.dossier_operation_logs.last }
-    let(:operation_serialized) { JSON.parse(last_operation.serialized.download) }
+    let(:operation_serialized) { last_operation.data }
     let(:instructeur) { create(:instructeur) }
 
     before { dossier.passer_en_instruction!(instructeur: instructeur) }
@@ -1123,7 +1123,7 @@ describe Dossier do
   describe '#passer_automatiquement_en_instruction!' do
     let(:dossier) { create(:dossier, :en_construction, en_construction_close_to_expiration_notice_sent_at: Time.zone.now) }
     let(:last_operation) { dossier.dossier_operation_logs.last }
-    let(:operation_serialized) { JSON.parse(last_operation.serialized.download) }
+    let(:operation_serialized) { last_operation.data }
     let(:instructeur) { create(:instructeur) }
 
     before { dossier.passer_automatiquement_en_instruction! }
@@ -1300,7 +1300,7 @@ describe Dossier do
     it { expect(dossier.attestation).to be_nil }
     it { expect(dossier.termine_close_to_expiration_notice_sent_at).to be_nil }
     it { expect(last_operation.operation).to eq('repasser_en_instruction') }
-    it { expect(JSON.parse(last_operation.serialized.download)['author']['email']).to eq(instructeur.email) }
+    it { expect(last_operation.data['author']['email']).to eq(instructeur.email) }
     it { expect(DossierMailer).to have_received(:notify_revert_to_instruction).with(dossier) }
 
     after { Timecop.return }

spec/services/procedure_archive_service_spec.rb

@@ -65,7 +65,7 @@ describe ProcedureArchiveService do
 
       context 'with a missing file' do
         let(:pj) do
-          PiecesJustificativesService::FakeAttachment.new(
+          ActiveStorage::FakeAttachment.new(
             file: StringIO.new('coucou'),
             filename: "export-dossier.pdf",
             name: 'pdf_export_for_instructeur',
@@ -75,7 +75,7 @@ describe ProcedureArchiveService do
         end
 
         let(:bad_pj) do
-          PiecesJustificativesService::FakeAttachment.new(
+          ActiveStorage::FakeAttachment.new(
             file: nil,
             filename: "cni.png",
             name: 'cni.png',