Merge pull request #2772 from projectblacklight/yaml-column-encoder-main

Use a custom YAML coder to restore backwards-compatible deserialization of serialized query parameters

.github/workflows/ruby.yml

@@ -33,20 +33,20 @@ jobs:
     strategy:
       matrix:
         ruby: [2.7, '3.0']
-        rails_version: ['6.1.5', '7.0.2.3']
+        rails_version: ['6.1.6.1', '7.0.3.1']
         bootstrap_version: [null]
         api: [null]
         additional_engine_cart_rails_options: ['']
         additional_name: ['']
         include:
         - ruby: '3.1'
-          rails_version: '7.0.2.3'
+          rails_version: '7.0.3.1'
         - ruby: '3.0'
-          rails_version: '7.0.2.3'
+          rails_version: '7.0.3.1'
           bootstrap_version: '~> 4.0'
           additional_name: '/ Bootstrap 4'
         - ruby: '3.0'
-          rails_version: '7.0.2.3'
+          rails_version: '7.0.3.1'
           api: 'true'
           additional_engine_cart_rails_options: -a propshaft --api --skip-yarn
           additional_name: '/ API'

app/models/search.rb

@@ -3,7 +3,7 @@
 class Search < ApplicationRecord
   belongs_to :user, optional: true
 
-  serialize :query_params
+  serialize :query_params, Blacklight::SearchParamsYamlCoder
 
   # A Search instance is considered a saved search if it has a user_id.
   def saved?

app/services/blacklight/search_params_yaml_coder.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+module Blacklight
+  # This is a custom YAML coder for (de)serializing blacklight search parameters that
+  # supports deserializing HashWithIndifferentAccess parameters (as was historically done by Blacklight).
+  class SearchParamsYamlCoder
+    # Serializes an attribute value to a string that will be stored in the database.
+    def self.dump(obj)
+      # Convert HWIA to an ordinary hash so we have some hope of using the regular YAML encoder in the future
+      obj = obj.to_hash if obj.is_a?(ActiveSupport::HashWithIndifferentAccess)
+
+      YAML.dump(obj)
+    end
+
+    # Deserializes a string from the database to an attribute value.
+    def self.load(yaml)
+      return yaml unless yaml.is_a?(String) && yaml.start_with?("---")
+
+      params = yaml_load(yaml)
+
+      params.with_indifferent_access
+    end
+
+    # rubocop:disable Security/YAMLLoad
+    if YAML.respond_to?(:unsafe_load)
+      def self.yaml_load(payload)
+        if ActiveRecord.try(:use_yaml_unsafe_load) || ActiveRecord::Base.try(:use_yaml_unsafe_load)
+          YAML.unsafe_load(payload)
+        else
+          permitted_classes = (ActiveRecord.try(:yaml_column_permitted_classes) || ActiveRecord::Base.try(:yaml_column_permitted_classes) || []) +
+                              Blacklight::Engine.config.blacklight.search_params_permitted_classes
+          YAML.safe_load(payload, permitted_classes: permitted_classes, aliases: true)
+        end
+      end
+    else
+      def self.yaml_load(payload)
+        if ActiveRecord.try(:use_yaml_unsafe_load) || ActiveRecord::Base.try(:use_yaml_unsafe_load)
+          YAML.load(payload)
+        else
+          permitted_classes = (ActiveRecord.try(:yaml_column_permitted_classes) || ActiveRecord::Base.try(:yaml_column_permitted_classes) || []) +
+                              Blacklight::Engine.config.blacklight.search_params_permitted_classes
+          YAML.safe_load(payload, permitted_classes: permitted_classes, aliases: true)
+        end
+      end
+    end
+    # rubocop:enable Security/YAMLLoad
+  end
+end

lib/blacklight/engine.rb

@@ -61,6 +61,8 @@ class Engine < Rails::Engine
       outer_window: 2
     }
 
+    bl_global_config.search_params_permitted_classes = [ActiveSupport::HashWithIndifferentAccess, Symbol]
+
     # Anything that goes into Blacklight::Engine.config is stored as a class
     # variable on Railtie::Configuration.  we're going to encapsulate all the
     # Blacklight specific stuff in this single struct:

spec/models/search_spec.rb

@@ -1,32 +1,51 @@
 # frozen_string_literal: true
 
 RSpec.describe Search do
+  let(:search) { described_class.new(user: user) }
   let(:user) { User.create! email: 'xyz@example.com', password: 'xyz12345' }
+  let(:hash_params) { { q: "query", f: { facet: "filter" } } }
+  let(:query_params) { hash_params }
 
   describe "query_params" do
-    before do
-      @search = described_class.new(user: user)
-      @query_params = { q: "query", f: "facet" }
+    shared_examples "persisting query_params" do
+      it "can save and retrieve the hash" do
+        search.query_params = query_params
+        search.save!
+        expect(described_class.find(search.id).query_params).to eq query_params.with_indifferent_access
+      end
     end
 
-    it "can save and retrieve the hash" do
-      @search.query_params = @query_params
-      @search.save!
-      expect(described_class.find(@search.id).query_params).to eq @query_params
+    context "are an indifferent hash" do
+      include_context "persisting query_params" do
+        let(:query_params) { hash_params.with_indifferent_access }
+      end
+    end
+
+    context "are a string-keyed hash" do
+      include_context "persisting query_params" do
+        let(:query_params) { hash_params.with_indifferent_access.to_hash }
+      end
+    end
+
+    context "include symbol keys" do
+      include_context "persisting query_params" do
+        let(:query_params) { hash_params }
+      end
     end
   end
 
   describe "saved?" do
     it "is true when user_id is not NULL and greater than 0" do
-      @search = described_class.new(user: user)
-      @search.save!
-
-      expect(@search).to be_saved
+      search.save!
+      expect(search).to be_saved
     end
 
-    it "is false when user_id is NULL or less than 1" do
-      @search = described_class.create
-      expect(@search).not_to be_saved
+    context "when user_id is NULL or less than 1" do
+      let(:search) { described_class.create }
+
+      it "is false" do
+        expect(search).not_to be_saved
+      end
     end
   end