From a98d88cb309707bab2790550ce6c56c08185822b Mon Sep 17 00:00:00 2001 From: Michael Raimondi Date: Mon, 6 Apr 2015 13:06:14 -0400 Subject: [PATCH 01/13] Use Postgres containment operator where available --- lib/paper_trail/version_concern.rb | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/lib/paper_trail/version_concern.rb b/lib/paper_trail/version_concern.rb index 32a1292c..528e879a 100644 --- a/lib/paper_trail/version_concern.rb +++ b/lib/paper_trail/version_concern.rb @@ -86,12 +86,17 @@ module PaperTrail # identically-named method in the serializer being used. def where_object(args = {}) raise ArgumentError, 'expected to receive a Hash' unless args.is_a?(Hash) - arel_field = arel_table[:object] - where_conditions = args.map do |field, value| - PaperTrail.serializer.where_object_condition(arel_field, field, value) - end.reduce do |condition1, condition2| - condition1.and(condition2) + if object_col_is_json? + where_conditions = "object @> '#{args.to_json}'::#{columns_hash['object'].type}" + else + arel_field = arel_table[:object] + + where_conditions = args.map do |field, value| + PaperTrail.serializer.where_object_condition(arel_field, field, value) + end.reduce do |condition1, condition2| + condition1.and(condition2) + end end where(where_conditions) @@ -99,12 +104,17 @@ module PaperTrail def where_object_changes(args = {}) raise ArgumentError, 'expected to receive a Hash' unless args.is_a?(Hash) - arel_field = arel_table[:object_changes] - where_conditions = args.map do |field, value| - PaperTrail.serializer.where_object_changes_condition(arel_field, field, value) - end.reduce do |condition1, condition2| - condition1.and(condition2) + if object_changes_col_is_json? + where_conditions = "object_changes @> '#{args.to_json}'::#{columns_hash['object_changes'].try(:type)}" + else + arel_field = arel_table[:object_changes] + + where_conditions = args.map do |field, value| + PaperTrail.serializer.where_object_changes_condition(arel_field, field, value) + end.reduce do |condition1, condition2| + condition1.and(condition2) + end end where(where_conditions) From 237f7f7f09e1f616b9421f875943b4a72e80827d Mon Sep 17 00:00:00 2001 From: Michael Raimondi Date: Thu, 9 Apr 2015 12:32:30 -0400 Subject: [PATCH 02/13] Update YAML engine check for new Ruby versions On Ruby 2.2.1, '::YAML' returns the class Psych, which has no ::ENGINE constant defined. --- lib/paper_trail/serializers/yaml.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/paper_trail/serializers/yaml.rb b/lib/paper_trail/serializers/yaml.rb index d4a02d42..fe59edf8 100644 --- a/lib/paper_trail/serializers/yaml.rb +++ b/lib/paper_trail/serializers/yaml.rb @@ -23,7 +23,8 @@ module PaperTrail # in the serialized object_changes def where_object_changes_condition(arel_field, field, value) # Need to check first (before) and secondary (after) fields - if defined?(::YAML::ENGINE) && ::YAML::ENGINE.yamler == 'psych' + if (defined?(::YAML::ENGINE) && ::YAML::ENGINE.yamler == 'psych') || + (defined?(::YAML) && defined?(Psych) && ::YAML == Psych) arel_field.matches("%\n#{field}:\n- #{value}\n%"). or(arel_field.matches("%\n#{field}:\n-%\n- #{value}\n%")) else # Syck adds extra spaces into array dumps From c08eee03ae158c895bd13929ef88365f91e2f8e3 Mon Sep 17 00:00:00 2001 From: Michael Raimondi Date: Thu, 9 Apr 2015 13:26:24 -0400 Subject: [PATCH 03/13] Do not memoize column type checks Column types can (rarely) change dynamically as an application runs. We should check the type of a column on every query. --- lib/paper_trail/version_concern.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/paper_trail/version_concern.rb b/lib/paper_trail/version_concern.rb index 528e879a..53abaaff 100644 --- a/lib/paper_trail/version_concern.rb +++ b/lib/paper_trail/version_concern.rb @@ -128,12 +128,12 @@ module PaperTrail # Returns whether the `object` column is using the `json` type supported by PostgreSQL def object_col_is_json? - @object_col_is_json ||= [:json, :jsonb].include?(columns_hash['object'].type) + [:json, :jsonb].include?(columns_hash['object'].type) end # Returns whether the `object_changes` column is using the `json` type supported by PostgreSQL def object_changes_col_is_json? - @object_changes_col_is_json ||= [:json, :jsonb].include?(columns_hash['object_changes'].try(:type)) + [:json, :jsonb].include?(columns_hash['object_changes'].try(:type)) end end From a7a7d0020c2afd76c064331040a0150d305e2c5e Mon Sep 17 00:00:00 2001 From: Michael Raimondi Date: Thu, 9 Apr 2015 13:48:35 -0400 Subject: [PATCH 04/13] Vary column type for tests if DB is postgres --- spec/models/version_spec.rb | 84 +++++++++++++++++++++++-------------- 1 file changed, 53 insertions(+), 31 deletions(-) diff --git a/spec/models/version_spec.rb b/spec/models/version_spec.rb index 3881e097..199a602f 100644 --- a/spec/models/version_spec.rb +++ b/spec/models/version_spec.rb @@ -65,46 +65,68 @@ describe PaperTrail::Version, :type => :model do end describe "Class" do - describe '#where_object' do - it { expect(PaperTrail::Version).to respond_to(:where_object) } - - context "invalid arguments" do - it "should raise an error" do - expect { PaperTrail::Version.where_object(:foo) }.to raise_error(ArgumentError) - expect { PaperTrail::Version.where_object([]) }.to raise_error(ArgumentError) - end - end - - context "valid arguments", :versioning => true do - let(:widget) { Widget.new } - let(:name) { Faker::Name.first_name } - let(:int) { rand(10) + 1 } + column_overrides = [false] + column_overrides.concat(['jsonb']) if ENV['DB'] == 'postgres' + column_overrides.each do |override| + context "with a #{override || 'text'} column" do before do - widget.update_attributes!(:name => name, :an_integer => int) - widget.update_attributes!(:name => 'foobar', :an_integer => 100) - widget.update_attributes!(:name => Faker::Name.last_name, :an_integer => 15) + if override + ActiveRecord::Base.connection.execute("SAVEPOINT pgtest;") + ActiveRecord::Base.connection.execute("ALTER TABLE versions DROP COLUMN object;") + ActiveRecord::Base.connection.execute("ALTER TABLE versions ADD COLUMN object #{override};") + PaperTrail::Version.reset_column_information + end end - - context "`serializer == YAML`" do - specify { expect(PaperTrail.serializer).to be PaperTrail::Serializers::YAML } - - it "should be able to locate versions according to their `object` contents" do - expect(PaperTrail::Version.where_object(:name => name)).to eq([widget.versions[1]]) - expect(PaperTrail::Version.where_object(:an_integer => 100)).to eq([widget.versions[2]]) + after do + if override + ActiveRecord::Base.connection.execute("ROLLBACK TO SAVEPOINT pgtest;") + PaperTrail::Version.reset_column_information end end - context "`serializer == JSON`" do - before(:all) { PaperTrail.serializer = PaperTrail::Serializers::JSON } - specify { expect(PaperTrail.serializer).to be PaperTrail::Serializers::JSON } + describe '#where_object' do + it { expect(PaperTrail::Version).to respond_to(:where_object) } - it "should be able to locate versions according to their `object` contents" do - expect(PaperTrail::Version.where_object(:name => name)).to eq([widget.versions[1]]) - expect(PaperTrail::Version.where_object(:an_integer => 100)).to eq([widget.versions[2]]) + context "invalid arguments" do + it "should raise an error" do + expect { PaperTrail::Version.where_object(:foo) }.to raise_error(ArgumentError) + expect { PaperTrail::Version.where_object([]) }.to raise_error(ArgumentError) + end end - after(:all) { PaperTrail.serializer = PaperTrail::Serializers::YAML } + context "valid arguments", :versioning => true do + let(:widget) { Widget.new } + let(:name) { Faker::Name.first_name } + let(:int) { rand(10) + 1 } + + before do + widget.update_attributes!(:name => name, :an_integer => int) + widget.update_attributes!(:name => 'foobar', :an_integer => 100) + widget.update_attributes!(:name => Faker::Name.last_name, :an_integer => 15) + end + + context "`serializer == YAML`" do + specify { expect(PaperTrail.serializer).to be PaperTrail::Serializers::YAML } + + it "should be able to locate versions according to their `object` contents" do + expect(PaperTrail::Version.where_object(:name => name)).to eq([widget.versions[1]]) + expect(PaperTrail::Version.where_object(:an_integer => 100)).to eq([widget.versions[2]]) + end + end + + context "`serializer == JSON`" do + before(:all) { PaperTrail.serializer = PaperTrail::Serializers::JSON } + specify { expect(PaperTrail.serializer).to be PaperTrail::Serializers::JSON } + + it "should be able to locate versions according to their `object` contents" do + expect(PaperTrail::Version.where_object(:name => name)).to eq([widget.versions[1]]) + expect(PaperTrail::Version.where_object(:an_integer => 100)).to eq([widget.versions[2]]) + end + + after(:all) { PaperTrail.serializer = PaperTrail::Serializers::YAML } + end + end end end end From f747711464a1e87d8b9b33dab6e5ff7ef7b816d2 Mon Sep 17 00:00:00 2001 From: Michael Raimondi Date: Thu, 9 Apr 2015 13:49:12 -0400 Subject: [PATCH 05/13] Containment operator is only available for JSONB columns --- lib/paper_trail/version_concern.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/paper_trail/version_concern.rb b/lib/paper_trail/version_concern.rb index 53abaaff..9f74b15e 100644 --- a/lib/paper_trail/version_concern.rb +++ b/lib/paper_trail/version_concern.rb @@ -87,8 +87,8 @@ module PaperTrail def where_object(args = {}) raise ArgumentError, 'expected to receive a Hash' unless args.is_a?(Hash) - if object_col_is_json? - where_conditions = "object @> '#{args.to_json}'::#{columns_hash['object'].type}" + if columns_hash['object'].type == :jsonb + where_conditions = "object @> '#{args.to_json}'::jsonb" else arel_field = arel_table[:object] @@ -105,8 +105,8 @@ module PaperTrail def where_object_changes(args = {}) raise ArgumentError, 'expected to receive a Hash' unless args.is_a?(Hash) - if object_changes_col_is_json? - where_conditions = "object_changes @> '#{args.to_json}'::#{columns_hash['object_changes'].try(:type)}" + if columns_hash['object'].type == :jsonb + where_conditions = "object_changes @> '#{args.to_json}'::jsonb" else arel_field = arel_table[:object_changes] From e52c53605ea82a1457e556add9c9f0291b9e1fe8 Mon Sep 17 00:00:00 2001 From: Michael Raimondi Date: Thu, 9 Apr 2015 14:24:09 -0400 Subject: [PATCH 06/13] Build query for JSON column type --- lib/paper_trail/version_concern.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/paper_trail/version_concern.rb b/lib/paper_trail/version_concern.rb index 9f74b15e..b2483db7 100644 --- a/lib/paper_trail/version_concern.rb +++ b/lib/paper_trail/version_concern.rb @@ -89,6 +89,11 @@ module PaperTrail if columns_hash['object'].type == :jsonb where_conditions = "object @> '#{args.to_json}'::jsonb" + elsif columns_hash['object'].type == :json + where_conditions = args.map do |field, value| + "object->>'#{field}' = '#{value}'" + end + where_conditions = where_conditions.join(" AND ") else arel_field = arel_table[:object] From 6d15205488ea79e0940aca5d88020e8137093dde Mon Sep 17 00:00:00 2001 From: Michael Raimondi Date: Thu, 9 Apr 2015 14:56:26 -0400 Subject: [PATCH 07/13] Add tests and refactor where_object_changes with JSONB --- lib/paper_trail/version_concern.rb | 3 +- spec/models/version_spec.rb | 104 +++++++++++++++-------------- 2 files changed, 55 insertions(+), 52 deletions(-) diff --git a/lib/paper_trail/version_concern.rb b/lib/paper_trail/version_concern.rb index b2483db7..087e0760 100644 --- a/lib/paper_trail/version_concern.rb +++ b/lib/paper_trail/version_concern.rb @@ -110,7 +110,8 @@ module PaperTrail def where_object_changes(args = {}) raise ArgumentError, 'expected to receive a Hash' unless args.is_a?(Hash) - if columns_hash['object'].type == :jsonb + if columns_hash['object_changes'].type == :jsonb + args.each { |field, value| args[field] = [value] } where_conditions = "object_changes @> '#{args.to_json}'::jsonb" else arel_field = arel_table[:object_changes] diff --git a/spec/models/version_spec.rb b/spec/models/version_spec.rb index 199a602f..4acca5f4 100644 --- a/spec/models/version_spec.rb +++ b/spec/models/version_spec.rb @@ -66,15 +66,17 @@ describe PaperTrail::Version, :type => :model do describe "Class" do column_overrides = [false] - column_overrides.concat(['jsonb']) if ENV['DB'] == 'postgres' + column_overrides.concat(%w[jsonb]) if ENV['DB'] == 'postgres' column_overrides.each do |override| context "with a #{override || 'text'} column" do before do if override ActiveRecord::Base.connection.execute("SAVEPOINT pgtest;") - ActiveRecord::Base.connection.execute("ALTER TABLE versions DROP COLUMN object;") - ActiveRecord::Base.connection.execute("ALTER TABLE versions ADD COLUMN object #{override};") + %w[object object_changes].each do |column| + ActiveRecord::Base.connection.execute("ALTER TABLE versions DROP COLUMN #{column};") + ActiveRecord::Base.connection.execute("ALTER TABLE versions ADD COLUMN #{column} #{override};") + end PaperTrail::Version.reset_column_information end end @@ -128,60 +130,60 @@ describe PaperTrail::Version, :type => :model do end end end - end - end - describe '#where_object_changes' do - it { expect(PaperTrail::Version).to respond_to(:where_object_changes) } + describe '#where_object_changes' do + it { expect(PaperTrail::Version).to respond_to(:where_object_changes) } - context "invalid arguments" do - it "should raise an error" do - expect { PaperTrail::Version.where_object_changes(:foo) }.to raise_error(ArgumentError) - expect { PaperTrail::Version.where_object_changes([]) }.to raise_error(ArgumentError) - end - end - - context "valid arguments", :versioning => true do - let(:widget) { Widget.new } - let(:name) { Faker::Name.first_name } - let(:int) { rand(5) + 2 } - - before do - widget.update_attributes!(:name => name, :an_integer => 0) - widget.update_attributes!(:name => 'foobar', :an_integer => 77) - widget.update_attributes!(:name => Faker::Name.last_name, :an_integer => int) - end - - context "`serializer == YAML`" do - specify { expect(PaperTrail.serializer).to be PaperTrail::Serializers::YAML } - - it "should be able to locate 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]) + context "invalid arguments" do + it "should raise an error" do + expect { PaperTrail::Version.where_object_changes(:foo) }.to raise_error(ArgumentError) + expect { PaperTrail::Version.where_object_changes([]) }.to raise_error(ArgumentError) + end end - it "should be able to handle queries for multiple attributes" do - expect(widget.versions.where_object_changes(:an_integer => 77, :name => 'foobar')).to eq(widget.versions[1..2]) + context "valid arguments", :versioning => true do + let(:widget) { Widget.new } + let(:name) { Faker::Name.first_name } + let(:int) { rand(5) + 2 } + + before do + widget.update_attributes!(:name => name, :an_integer => 0) + widget.update_attributes!(:name => 'foobar', :an_integer => 77) + widget.update_attributes!(:name => Faker::Name.last_name, :an_integer => int) + end + + context "`serializer == YAML`" do + specify { expect(PaperTrail.serializer).to be PaperTrail::Serializers::YAML } + + it "should be able to locate 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 "should be able to handle queries for multiple attributes" do + expect(widget.versions.where_object_changes(:an_integer => 77, :name => 'foobar')).to eq(widget.versions[1..2]) + end + end + + context "`serializer == JSON`" do + before(:all) { PaperTrail.serializer = PaperTrail::Serializers::JSON } + specify { expect(PaperTrail.serializer).to be PaperTrail::Serializers::JSON } + + it "should be able to locate 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 "should be able to handle queries for multiple attributes" do + expect(widget.versions.where_object_changes(:an_integer => 77, :name => 'foobar')).to eq(widget.versions[1..2]) + end + + after(:all) { PaperTrail.serializer = PaperTrail::Serializers::YAML } + end end end - - context "`serializer == JSON`" do - before(:all) { PaperTrail.serializer = PaperTrail::Serializers::JSON } - specify { expect(PaperTrail.serializer).to be PaperTrail::Serializers::JSON } - - it "should be able to locate 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 "should be able to handle queries for multiple attributes" do - expect(widget.versions.where_object_changes(:an_integer => 77, :name => 'foobar')).to eq(widget.versions[1..2]) - end - - after(:all) { PaperTrail.serializer = PaperTrail::Serializers::YAML } - end end end end From 5dc53603912dfb49caac011e1b5ea4878f99cc00 Mon Sep 17 00:00:00 2001 From: Michael Raimondi Date: Thu, 9 Apr 2015 15:27:15 -0400 Subject: [PATCH 08/13] Implement where_object_changes query for JSON columns --- lib/paper_trail/version_concern.rb | 5 +++++ spec/models/version_spec.rb | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/paper_trail/version_concern.rb b/lib/paper_trail/version_concern.rb index 087e0760..04f673d0 100644 --- a/lib/paper_trail/version_concern.rb +++ b/lib/paper_trail/version_concern.rb @@ -113,6 +113,11 @@ module PaperTrail if columns_hash['object_changes'].type == :jsonb args.each { |field, value| args[field] = [value] } where_conditions = "object_changes @> '#{args.to_json}'::jsonb" + elsif columns_hash['object'].type == :json + where_conditions = args.map do |field, value| + "((object_changes->>'#{field}' ILIKE '[#{value.to_json},%') OR (object_changes->>'#{field}' ILIKE '[%,#{value.to_json}]%'))" + end + where_conditions = where_conditions.join(" AND ") else arel_field = arel_table[:object_changes] diff --git a/spec/models/version_spec.rb b/spec/models/version_spec.rb index 4acca5f4..5cfb4b18 100644 --- a/spec/models/version_spec.rb +++ b/spec/models/version_spec.rb @@ -66,9 +66,9 @@ describe PaperTrail::Version, :type => :model do describe "Class" do column_overrides = [false] - column_overrides.concat(%w[jsonb]) if ENV['DB'] == 'postgres' + column_overrides.concat(%w[json jsonb]) if ENV['DB'] == 'postgres' - column_overrides.each do |override| + column_overrides.shuffle.each do |override| context "with a #{override || 'text'} column" do before do if override From 5a408e20da4551adb8bdbd9050ce1a56aa288a6d Mon Sep 17 00:00:00 2001 From: Michael Raimondi Date: Thu, 9 Apr 2015 15:34:58 -0400 Subject: [PATCH 09/13] Bump Postgres version in CI --- .travis.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 963b1588..f8e58dfa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -37,4 +37,6 @@ matrix: gemfile: Gemfile - rvm: 1.8.7 gemfile: Gemfile - \ No newline at end of file + +addons: + postgresql: "9.4" From 7834309857213f3ca9db9b3af8285fd09d0d28f6 Mon Sep 17 00:00:00 2001 From: Ben Atkins Date: Wed, 6 May 2015 22:29:39 -0400 Subject: [PATCH 10/13] Add test cases explicitly surrounding JSON type columns when the test suite is executed against Postgres --- spec/models/json_version_spec.rb | 78 +++++++++++++++++++ test/dummy/app/models/fruit.rb | 5 ++ test/dummy/app/versions/json_version.rb | 3 + .../20110208155312_set_up_test_tables.rb | 24 ++++++ 4 files changed, 110 insertions(+) create mode 100644 spec/models/json_version_spec.rb create mode 100644 test/dummy/app/models/fruit.rb create mode 100644 test/dummy/app/versions/json_version.rb diff --git a/spec/models/json_version_spec.rb b/spec/models/json_version_spec.rb new file mode 100644 index 00000000..c714b379 --- /dev/null +++ b/spec/models/json_version_spec.rb @@ -0,0 +1,78 @@ +require 'rails_helper' + +describe JsonVersion, :type => :model do + it "should include the `VersionConcern` module to get base functionality" do + expect(JsonVersion).to include(PaperTrail::VersionConcern) + end + + if ENV['DB'] == 'postgres' || JsonVersion.table_exists? + describe "Methods" do + describe "Class" do + + describe '#where_object' do + it { expect(JsonVersion).to respond_to(:where_object) } + + context "invalid arguments" do + it "should raise an error" do + expect { JsonVersion.where_object(:foo) }.to raise_error(ArgumentError) + expect { JsonVersion.where_object([]) }.to raise_error(ArgumentError) + end + end + + context "valid arguments", :versioning => true do + let(:fruit_names) { %w(apple orange lemon banana lime coconut strawberry blueberry) } + let(:fruit) { Fruit.new } + let(:name) { 'pomegranate' } + let(:color) { Faker::Color.name } + + before do + fruit.update_attributes!(:name => name) + fruit.update_attributes!(:name => fruit_names.sample, :color => color) + fruit.update_attributes!(:name => fruit_names.sample, :color => Faker::Color.name) + end + + it "should be able to locate versions according to their `object` contents" do + expect(JsonVersion.where_object(:name => name)).to eq([fruit.versions[1]]) + expect(JsonVersion.where_object(:color => color)).to eq([fruit.versions[2]]) + end + end + end + + describe '#where_object_changes' do + it { expect(JsonVersion).to respond_to(:where_object_changes) } + + context "invalid arguments" do + it "should raise an error" do + expect { JsonVersion.where_object_changes(:foo) }.to raise_error(ArgumentError) + expect { JsonVersion.where_object_changes([]) }.to raise_error(ArgumentError) + end + end + + context "valid arguments", :versioning => true do + let(:fruit_names) { %w(apple orange lemon banana lime strawberry blueberry) } + let(:tropical_fruit_names) { %w(coconut pineapple kiwi mango melon) } + let(:fruit) { Fruit.new } + let(:name) { 'pomegranate' } + let(:color) { Faker::Color.name } + + before do + fruit.update_attributes!(:name => name) + fruit.update_attributes!(:name => tropical_fruit_names.sample, :color => color) + fruit.update_attributes!(:name => fruit_names.sample, :color => Faker::Color.name) + end + + it "should be able to locate versions according to their `object_changes` contents" do + expect(fruit.versions.where_object_changes(:name => name)).to eq(fruit.versions[0..1]) + expect(fruit.versions.where_object_changes(:color => color)).to eq(fruit.versions[1..2]) + end + + it "should be able to handle queries for multiple attributes" do + expect(fruit.versions.where_object_changes(:color => color, :name => name)).to eq([fruit.versions[1]]) + end + end + end + + end + end + end +end diff --git a/test/dummy/app/models/fruit.rb b/test/dummy/app/models/fruit.rb new file mode 100644 index 00000000..644e195a --- /dev/null +++ b/test/dummy/app/models/fruit.rb @@ -0,0 +1,5 @@ +class Fruit < ActiveRecord::Base + if ENV['DB'] == 'postgres' || JsonVersion.table_exists? + has_paper_trail :class_name => 'JsonVersion' + end +end diff --git a/test/dummy/app/versions/json_version.rb b/test/dummy/app/versions/json_version.rb new file mode 100644 index 00000000..3a6ac021 --- /dev/null +++ b/test/dummy/app/versions/json_version.rb @@ -0,0 +1,3 @@ +class JsonVersion < PaperTrail::Version + self.table_name = 'json_versions' +end diff --git a/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb b/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb index 9cba09fc..e0784ea9 100644 --- a/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb +++ b/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb @@ -60,6 +60,19 @@ class SetUpTestTables < ActiveRecord::Migration end add_index :post_versions, [:item_type, :item_id] + if ENV['DB'] == 'postgres' + create_table :json_versions, :force => true do |t| + t.string :item_type, :null => false + t.integer :item_id, :null => false + t.string :event, :null => false + t.string :whodunnit + t.json :object + t.json :object_changes + t.datetime :created_at + end + add_index :json_versions, [:item_type, :item_id] + end + create_table :wotsits, :force => true do |t| t.integer :widget_id t.string :name @@ -164,6 +177,11 @@ class SetUpTestTables < ActiveRecord::Migration t.integer :order_id t.string :product end + + create_table :fruits, :force => true do |t| + t.string :name + t.string :color + end end def self.down @@ -183,14 +201,20 @@ class SetUpTestTables < ActiveRecord::Migration drop_table :post_versions remove_index :versions, :column => [:item_type, :item_id] drop_table :versions + if JsonVersion.table_exists? + remove_index :json_versions, :column => [:item_type, :item_id] + drop_table :json_versions + end drop_table :widgets drop_table :documents drop_table :legacy_widgets + drop_table :things drop_table :translations drop_table :gadgets drop_table :customers drop_table :orders drop_table :line_items + drop_table :fruits remove_index :version_associations, :column => [:version_id] remove_index :version_associations, :name => 'index_version_associations_on_foreign_key' drop_table :version_associations From 46f1709ab3bc47ad263c75afc66c8df8924c4b79 Mon Sep 17 00:00:00 2001 From: Ben Atkins Date: Thu, 7 May 2015 14:45:03 -0400 Subject: [PATCH 11/13] Add ActiveRecord version limitation for the tests against jsonb column types since only ActiveRecord 4.2+ has support for it --- spec/models/version_spec.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/spec/models/version_spec.rb b/spec/models/version_spec.rb index 5cfb4b18..153ad092 100644 --- a/spec/models/version_spec.rb +++ b/spec/models/version_spec.rb @@ -66,7 +66,11 @@ describe PaperTrail::Version, :type => :model do describe "Class" do column_overrides = [false] - column_overrides.concat(%w[json jsonb]) if ENV['DB'] == 'postgres' + if ENV['DB'] == 'postgres' + column_overrides << 'json' + # 'jsonb' column types are only supported for ActiveRecord 4.2+ + column_overrides << 'jsonb' if ::ActiveRecord::VERSION::STRING >= '4.2' + end column_overrides.shuffle.each do |override| context "with a #{override || 'text'} column" do From 916b78897b1956125e55ee058458cefea49001bb Mon Sep 17 00:00:00 2001 From: Ben Atkins Date: Thu, 7 May 2015 14:56:28 -0400 Subject: [PATCH 12/13] Only execute the JsonVersion tests against ActiveRecord4 since there is no ActiveRecord3 support for it --- spec/models/json_version_spec.rb | 16 +++++++++------- .../migrate/20110208155312_set_up_test_tables.rb | 2 +- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/spec/models/json_version_spec.rb b/spec/models/json_version_spec.rb index c714b379..ac6b0cfc 100644 --- a/spec/models/json_version_spec.rb +++ b/spec/models/json_version_spec.rb @@ -1,11 +1,12 @@ -require 'rails_helper' +if JsonVersion.table_exists? -describe JsonVersion, :type => :model do - it "should include the `VersionConcern` module to get base functionality" do - expect(JsonVersion).to include(PaperTrail::VersionConcern) - end + require 'rails_helper' + + describe JsonVersion, :type => :model do + it "should include the `VersionConcern` module to get base functionality" do + expect(JsonVersion).to include(PaperTrail::VersionConcern) + end - if ENV['DB'] == 'postgres' || JsonVersion.table_exists? describe "Methods" do describe "Class" do @@ -71,8 +72,9 @@ describe JsonVersion, :type => :model do end end end - end + end end + end diff --git a/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb b/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb index e0784ea9..1f7f36e0 100644 --- a/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb +++ b/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb @@ -60,7 +60,7 @@ class SetUpTestTables < ActiveRecord::Migration end add_index :post_versions, [:item_type, :item_id] - if ENV['DB'] == 'postgres' + if ENV['DB'] == 'postgres' && ::ActiveRecord::VERSION::MAJOR >= 4 create_table :json_versions, :force => true do |t| t.string :item_type, :null => false t.integer :item_id, :null => false From 936239e552da358b76f25dbf979e71b870cd20b7 Mon Sep 17 00:00:00 2001 From: Ben Atkins Date: Thu, 7 May 2015 15:04:15 -0400 Subject: [PATCH 13/13] There is no JSON column type support in ActiveRecord3 --- spec/models/version_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/version_spec.rb b/spec/models/version_spec.rb index 153ad092..26e815dd 100644 --- a/spec/models/version_spec.rb +++ b/spec/models/version_spec.rb @@ -66,7 +66,7 @@ describe PaperTrail::Version, :type => :model do describe "Class" do column_overrides = [false] - if ENV['DB'] == 'postgres' + if ENV['DB'] == 'postgres' && ::ActiveRecord::VERSION::MAJOR >= 4 column_overrides << 'json' # 'jsonb' column types are only supported for ActiveRecord 4.2+ column_overrides << 'jsonb' if ::ActiveRecord::VERSION::STRING >= '4.2'