diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index 121710679..c521f2a37 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -76,11 +76,11 @@ jobs: - name: Install dependencies run: bundle install env: - RAILS_VERSION: 6.0.3.7 + RAILS_VERSION: 6.0.5.1 - name: Run tests run: bundle exec rake ci env: - RAILS_VERSION: 6.0.3.7 + RAILS_VERSION: 6.0.5.1 ENGINE_CART_RAILS_OPTIONS: '--skip-git --skip-listen --skip-spring --skip-keeps --skip-action-cable --skip-coffee --skip-test' test_rails5_2: runs-on: ubuntu-latest @@ -96,11 +96,11 @@ jobs: - name: Install dependencies run: bundle install env: - RAILS_VERSION: 5.2.4.6 + RAILS_VERSION: 5.2.8.1 - name: Run tests run: bundle exec rake ci env: - RAILS_VERSION: 5.2.4.6 + RAILS_VERSION: 5.2.8.1 ENGINE_CART_RAILS_OPTIONS: '--skip-git --skip-listen --skip-spring --skip-keeps --skip-action-cable --skip-coffee --skip-test' test_rails6_1: @@ -117,11 +117,11 @@ jobs: - name: Install dependencies run: bundle install env: - RAILS_VERSION: 6.1.5 + RAILS_VERSION: 6.1.6.1 - name: Run tests run: bundle exec rake ci env: - RAILS_VERSION: 6.1.5 + RAILS_VERSION: 6.1.6.1 ENGINE_CART_RAILS_OPTIONS: '--skip-git --skip-keeps --skip-action-cable --skip-test' api_test: runs-on: ubuntu-latest diff --git a/app/models/search.rb b/app/models/search.rb index 9eae75c2e..02f50edaa 100644 --- a/app/models/search.rb +++ b/app/models/search.rb @@ -3,7 +3,8 @@ class Search < ApplicationRecord belongs_to :user, optional: true - serialize :query_params + # use a backwards-compatible serializer until the Rails API stabilizes and we can evaluate for major-revision compatibility + serialize :query_params, Blacklight::SearchParamsYamlCoder # A Search instance is considered a saved search if it has a user_id. def saved? diff --git a/app/services/blacklight/search_params_yaml_coder.rb b/app/services/blacklight/search_params_yaml_coder.rb new file mode 100644 index 000000000..a5daffa79 --- /dev/null +++ b/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 diff --git a/lib/blacklight/engine.rb b/lib/blacklight/engine.rb index efcb877a9..3cf0202e0 100644 --- a/lib/blacklight/engine.rb +++ b/lib/blacklight/engine.rb @@ -59,6 +59,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: diff --git a/spec/models/search_spec.rb b/spec/models/search_spec.rb index 92aa27a7f..1fac93081 100644 --- a/spec/models/search_spec.rb +++ b/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