Msgpack metadata (#1553)

* Serialize metadata with MessagePack

* Bump bootsnap to 1.18

---------

Co-authored-by: noracato <[email protected]>

Author: louispt1

Gemfile.lock

@@ -180,7 +180,7 @@ GEM
     bindata (2.5.0)
     binding_of_caller (1.0.0)
       debug_inspector (>= 0.0.1)
-    bootsnap (1.10.3)
+    bootsnap (1.18.4)
       msgpack (~> 1.2)
     builder (3.3.0)
     byebug (11.1.3)

app/models/scenario.rb

@@ -15,8 +15,8 @@ class Scenario < ApplicationRecord
 
   serialize  :user_values, type: Hash, coder: MessagePack
   serialize :balanced_values, type: Hash, coder: MessagePack
-  serialize :active_couplings, type: Array
-  store :metadata, coder: JSON
+  serialize  :active_couplings, type: Array
+  serialize  :metadata, type: Hash, coder: MessagePack
 
   has_many   :scenario_users, dependent: :destroy
   has_many   :users, through: :scenario_users

app/views/inspect/scenarios/index.html.haml

@@ -39,8 +39,8 @@
             %span.tag.gray Public
         %td
           = link_to s.id, inspect_scenario_path(:id => s.id)
-          - if s.metadata[:title]
-            %span{ style: 'margin: 0 0.25rem 0 0.5rem; font-weight: normal' }= s.metadata[:title]
+          - if s.title
+            %span{ style: 'margin: 0 0.25rem 0 0.5rem; font-weight: normal' }= s.title
           - if s.id.to_s == params[:api_scenario_id]
             %span.tag.green ★ Current Scenario
         %td

config/initializers/msgpack.rb

@@ -3,6 +3,6 @@
 module MessagePack
   class << self
     alias_method :load, :unpack
-    alias_method :dump,  :pack
+    alias_method :dump, :pack
   end
 end

db/migrate/20250410135346_serialize_metadata.rb

@@ -0,0 +1,34 @@
+class SerializeMetadata < ActiveRecord::Migration[7.1]
+  include ETEngine::ScenarioMigration
+
+  class ScenarioForMigration < ActiveRecord::Base
+    self.table_name = 'scenarios'
+  end
+
+  require 'msgpack'
+
+  def up
+    change_column :scenarios, :metadata, :text, size: :medium
+    add_column :scenarios, :metadata_binary, :binary, limit:64.kilobytes
+
+    migrate_scenarios(raise_if_no_changes: false) do |scenario|
+      original = ScenarioForMigration.find(scenario.id)
+      yaml_data = original.read_attribute_before_type_cast("metadata")
+      next if yaml_data.blank? || !yaml_data.is_a?(String)
+
+      begin
+        # Had to add other permitted_classes
+        values = YAML.safe_load(yaml_data, permitted_classes: [Hash, Float, Integer, String, Symbol], aliases: true)
+        next unless values.is_a?(Hash)
+        msgpack_blob = values.to_h.to_msgpack
+
+        ScenarioForMigration.find(scenario.id).update!(metadata_binary: msgpack_blob)
+      rescue => e
+        Rails.logger.warn("Skipping scenario ##{scenario.id}: #{e.message}")
+      end
+    end
+
+    rename_column :scenarios, :metadata, :metadata_old
+    rename_column :scenarios, :metadata_binary, :metadata
+  end
+end

db/schema.rb

@@ -126,10 +126,11 @@
     t.string "area_code"
     t.string "source"
     t.text "balanced_values_old", size: :medium
-    t.text "metadata"
+    t.text "metadata_old", size: :medium
     t.text "active_couplings", size: :medium
     t.binary "user_values", size: :long
     t.binary "balanced_values", size: :medium
+    t.binary "metadata", size: :medium
     t.index ["created_at"], name: "index_scenarios_on_created_at"
   end
 

spec/controllers/api/v3/scenarios_controller_spec.rb

@@ -145,7 +145,7 @@
 
       it 'makes the ctm_scenario_id available' do
         scenario = Scenario.find(JSON.parse(response.body)['id'])
-        expect(scenario.metadata[:ctm_scenario_id]).to eq('123')
+        expect(scenario.metadata["ctm_scenario_id"]).to eq('123')
       end
 
       context 'when metadata is huge' do

spec/models/scenario_spec.rb

@@ -737,38 +737,38 @@
 
     context 'with no metadata' do
       it 'responds with nil when requesting a key' do
-        expect(scenario.metadata[:ctm_scenario_id]).to be_nil
+        expect(scenario.metadata["ctm_scenario_id"]).to be_nil
       end
     end
 
     context 'with empty metadata' do
       before { scenario.metadata = {} }
 
       it 'responds with nil when requesting a key' do
-        expect(scenario.metadata[:ctm_scenario_id]).to be_nil
+        expect(scenario.metadata["ctm_scenario_id"]).to be_nil
       end
     end
 
     context 'with metadata present' do
-      before { scenario.metadata = { ctm_scenario_id: 12_345, kittens: 'mew' } }
+      before { scenario.metadata = { "ctm_scenario_id" => 12_345, "kittens" => "mew" } }
 
       it 'stores numeric data' do
-        expect(scenario.metadata[:ctm_scenario_id]).to eq(12_345)
+        expect(scenario.metadata["ctm_scenario_id"]).to eq(12_345)
       end
 
       it 'stores string data' do
-        expect(scenario.metadata[:kittens]).to eq('mew')
+        expect(scenario.metadata["kittens"]).to eq("mew")
       end
 
-      it 'does not have metadata accesible by accessor' do
+      it 'does not have metadata accessible by accessor' do
         expect { scenario.kittens }.to raise_error(NoMethodError)
       end
     end
 
     context 'when setting metadata' do
-      it 'permits JSON object' do
-        scenario.metadata = JSON.generate({})
-        expect(scenario.metadata).to eq({})
+      it 'permits a hash' do
+        scenario.metadata = { "a" => 1 }
+        expect(scenario.metadata).to eq({ "a" => 1 })
       end
 
       it 'permits a hash' do
@@ -781,21 +781,14 @@
         expect(scenario.metadata).to eq({})
       end
 
-      it 'permits empty string' do
-        scenario.metadata = ''
-        expect(scenario.metadata).to eq({})
-      end
-
       it 'denies objects larger than 64Kb' do
-        scenario.metadata = (0..15_000).to_h { |i| [i, i] }
-
+        scenario.metadata = (0..15_000).to_h { |i| [i.to_s, i] }
         expect(scenario).not_to be_valid
       end
     end
 
     context 'when creating a clone of a scenario' do
-      before { scenario.metadata = { ctm_scenario_id: 12_345, kittens: 'mew' } }
-
+      before { scenario.metadata = { "ctm_scenario_id" => 12_345, "kittens" => "mew" } }
       let(:scenario_clone) { described_class.new(scenario_id: scenario.id) }
 
       it 'keeps the original metadata' do