From 3e0669b5edb488f530e820c11f572b63f4d5ad83 Mon Sep 17 00:00:00 2001 From: Jared Beck Date: Wed, 20 Sep 2017 03:53:51 -0400 Subject: [PATCH] where_object_changes: reading json from text column: raise error Breaking change. --- CHANGELOG.md | 3 +- lib/paper_trail/serializers/json.rb | 26 ++++------ spec/models/version_spec.rb | 58 +++++++++++------------ spec/paper_trail/serializers/json_spec.rb | 8 ++++ 4 files changed, 46 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44415a09..29f79df8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,8 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/). ### Breaking Changes -- None +- [#803](https://github.com/airblade/paper_trail/issues/803) - + where_object_changes no longer supports reading json from a text column ### Added diff --git a/lib/paper_trail/serializers/json.rb b/lib/paper_trail/serializers/json.rb index 908eeaf3..c71cacba 100644 --- a/lib/paper_trail/serializers/json.rb +++ b/lib/paper_trail/serializers/json.rb @@ -4,13 +4,6 @@ module PaperTrail module Serializers # An alternate serializer for, e.g. `versions.object`. module JSON - E_WHERE_OBJ_CHANGES = <<-STR.squish.freeze - where_object_changes has a known issue. When reading json from a text - column, it may return more records than expected. Instead of a warning, - this method may raise an error in the future. Please join the discussion - at https://github.com/airblade/paper_trail/issues/803 - STR - extend self # makes all instance methods become module methods as well def load(string) @@ -39,17 +32,14 @@ module PaperTrail end end - # Returns a SQL LIKE condition to be used to match the given field and - # value in the serialized `object_changes`. - def where_object_changes_condition(arel_field, field, value) - ::ActiveSupport::Deprecation.warn(E_WHERE_OBJ_CHANGES) - - # Convert to JSON to handle strings and nulls correctly. - json_value = value.to_json - - # Need to check first (before) and secondary (after) fields - arel_field.matches("%\"#{field}\":[#{json_value},%"). - or(arel_field.matches("%\"#{field}\":[%,#{json_value}]%")) + def where_object_changes_condition(*) + raise <<-STR.squish.freeze + where_object_changes no longer supports reading json from a text + column. The old implementation was inaccurate, returning more records + than you wanted. This feature was deprecated in 7.1.0 and removed in + 8.0.0. The json and jsonb datatypes are still supported. See the + discussion at https://github.com/airblade/paper_trail/issues/803 + STR end end end diff --git a/spec/models/version_spec.rb b/spec/models/version_spec.rb index 3621be4e..9d36d6b9 100644 --- a/spec/models/version_spec.rb +++ b/spec/models/version_spec.rb @@ -105,10 +105,10 @@ module PaperTrail column_overrides << "jsonb" if ::ActiveRecord::VERSION::STRING >= "4.2" end - column_overrides.shuffle.each do |override| - context "with a #{override || 'text'} column" do + column_overrides.shuffle.each do |column_datatype_override| + context "with a #{column_datatype_override || 'text'} column" do before do - if override + if column_datatype_override # In rails < 5, we use truncation, ie. there is no transaction # around the tests, so we can't use a savepoint. if active_record_gem_version >= ::Gem::Version.new("5") @@ -119,7 +119,7 @@ module PaperTrail "ALTER TABLE versions DROP COLUMN #{column};" ) ActiveRecord::Base.connection.execute( - "ALTER TABLE versions ADD COLUMN #{column} #{override};" + "ALTER TABLE versions ADD COLUMN #{column} #{column_datatype_override};" ) end PaperTrail::Version.reset_column_information @@ -127,7 +127,7 @@ module PaperTrail end after do - if override + if column_datatype_override # In rails < 5, we use truncation, ie. there is no transaction # around the tests, so we can't use a savepoint. if active_record_gem_version >= ::Gem::Version.new("5") @@ -245,35 +245,33 @@ module PaperTrail end end - context "JSON serializer" do - before(:all) { PaperTrail.serializer = PaperTrail::Serializers::JSON } - before do - unless override - expect(::ActiveSupport::Deprecation).to( - receive(:warn).at_least(:once).with(/^where_object_changes/) - ) + # Only test the JSON serializer against where_object_changes + # for json and jsonb columns, not for text columns, which are + # no longer supported. + if column_datatype_override + context "JSON serializer" do + before(:all) { PaperTrail.serializer = PaperTrail::Serializers::JSON } + + it "locates versions according to their `object_changes` contents" do + expect( + widget.versions.where_object_changes(name: name) + ).to eq(widget.versions[0..1]) + expect( + widget.versions.where_object_changes(an_integer: 77) + ).to eq(widget.versions[1..2]) + expect( + widget.versions.where_object_changes(an_integer: int) + ).to eq([widget.versions.last]) end - end - it "locates versions according to their `object_changes` contents" do - expect( - widget.versions.where_object_changes(name: name) - ).to eq(widget.versions[0..1]) - expect( - widget.versions.where_object_changes(an_integer: 77) - ).to eq(widget.versions[1..2]) - expect( - widget.versions.where_object_changes(an_integer: int) - ).to eq([widget.versions.last]) - end + it "handles queries for multiple attributes" do + expect( + widget.versions.where_object_changes(an_integer: 77, name: "foobar") + ).to eq(widget.versions[1..2]) + end - it "handles queries for multiple attributes" do - expect( - widget.versions.where_object_changes(an_integer: 77, name: "foobar") - ).to eq(widget.versions[1..2]) + after(:all) { PaperTrail.serializer = PaperTrail::Serializers::YAML } end - - after(:all) { PaperTrail.serializer = PaperTrail::Serializers::YAML } end end end diff --git a/spec/paper_trail/serializers/json_spec.rb b/spec/paper_trail/serializers/json_spec.rb index 28be37d6..24f14de1 100644 --- a/spec/paper_trail/serializers/json_spec.rb +++ b/spec/paper_trail/serializers/json_spec.rb @@ -52,6 +52,14 @@ module PaperTrail end end end + + describe ".where_object_changes_condition" do + it "raises error" do + expect { + described_class.where_object_changes_condition + }.to raise_error(/no longer supports/) + end + end end end end