From 23a59e3654fe990029024d3de243afb0fb71d4af Mon Sep 17 00:00:00 2001 From: Weston Ganger Date: Tue, 12 Nov 2024 00:23:04 -0800 Subject: [PATCH] Use native_json as default storage method for new installations (#67) - Switch default storage method to native SQL JSON columns - No longer recommend to set `ActiveSnapshot.config.storage_method`, its only retained to support legacy installations - Drop support for Rails 6.0 --- .github/workflows/test.yml | 16 ++++------ .gitignore | 1 + CHANGELOG.md | 3 +- README.md | 19 ------------ active_snapshot.gemspec | 2 +- lib/active_snapshot/config.rb | 30 +++++++++++++------ lib/active_snapshot/models/snapshot.rb | 12 +++++--- lib/active_snapshot/models/snapshot_item.rb | 14 +++++---- .../templates/create_snapshots_tables.rb.erb | 4 +-- test/models/snapshot_item_test.rb | 4 +-- test/models/snapshot_test.rb | 4 +-- test/test_helper.rb | 28 ++++------------- test/unit/config_test.rb | 29 ++++++++++++------ 13 files changed, 77 insertions(+), 89 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b1e083a..78a10a6 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -25,8 +25,6 @@ jobs: - ruby: "3.2" - ruby: "3.3" ### TEST RAILS VERSIONS - - ruby: "2.6" - rails_version: "~> 6.0.0" - ruby: "2.6" rails_version: "~> 6.1.0" - ruby: "3.3" @@ -38,6 +36,11 @@ jobs: rails_version: "~> 7.2.0" - ruby: "3.3" rails_version: "~> 8.0.0" + ### DB TESTING TESTING + - ruby: 3.3 + db_gem: "mysql2" + - ruby: 3.3 + db_gem: "pg" ### STORAGE METHOD TESTING - ruby: 3.3 db_gem: "sqlite3" @@ -45,27 +48,18 @@ jobs: - ruby: 3.3 db_gem: "sqlite3" active_snapshot_storage_method: "serialized_yaml" - - ruby: 3.3 - db_gem: "sqlite3" - active_snapshot_storage_method: "native_json" - ruby: 3.3 db_gem: "mysql2" active_snapshot_storage_method: "serialized_json" - ruby: 3.3 db_gem: "mysql2" active_snapshot_storage_method: "serialized_yaml" - - ruby: 3.3 - db_gem: "mysql2" - active_snapshot_storage_method: "native_json" - ruby: 3.3 db_gem: "pg" active_snapshot_storage_method: "serialized_json" - ruby: 3.3 db_gem: "pg" active_snapshot_storage_method: "serialized_yaml" - - ruby: 3.3 - db_gem: "pg" - active_snapshot_storage_method: "native_json" services: mysql: diff --git a/.gitignore b/.gitignore index 9fe3c00..ec40647 100644 --- a/.gitignore +++ b/.gitignore @@ -16,4 +16,5 @@ Gemfile.lock /test/**/tmp/ test/dummy_app/**/*.sqlite* +test/dummy_app/db/schema.rb test/dummy_app/**/*.log diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ec1836..6e76c4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,8 @@ CHANGELOG - **Unreleased** * [View Diff](https://github.com/westonganger/active_snapshot/compare/v0.5.1...master) - * Nothing yet + * [#67](https://github.com/westonganger/active_snapshot/pull/67) - Switch default storage method to native SQL JSON columns. No longer recommend to set `ActiveSnapshot.config.storage_method`, its only retained to support legacy installations + * Drop support for Rails 6.0. Rails 6.1 is minimum required version now. - **v0.5.1** - Nov 11, 2024 * [View Diff](https://github.com/westonganger/active_snapshot/compare/v0.5.0...v0.5.1) diff --git a/README.md b/README.md index 368c76b..35385d8 100644 --- a/README.md +++ b/README.md @@ -53,25 +53,6 @@ It defines an optional extension to your model: `has_snapshot_children`. It defines one instance method to your model: `create_snapshot!` -# Using a different storage format - -By default ActiveSnapshot encodes objects to JSON and stores in the database as plain text. If you prefer to have YAML encoded columns or native JSON DB columns you can configure this as follows: - -```ruby -ActiveSnapshot.config do |config| - config.storage_method = "serialized_json" # default, for text column - #config.storage_method = "serialized_yaml" # for text column - #config.storage_method = "native_json" # for json/jsonb column -end -``` - -If using a native json column, you should configure the `storage_method` before generating the migration. If this step was missed then you would need to create a migration to change the `:object` and `:metadata` columns to json (or jsonb) - -```ruby -change_column :snapshots, :object, :json -change_column :snapshots, :metadata, :json -``` - # Basic Usage You now have access to the following methods: diff --git a/active_snapshot.gemspec b/active_snapshot.gemspec index cf56b9b..8fe1f37 100644 --- a/active_snapshot.gemspec +++ b/active_snapshot.gemspec @@ -17,7 +17,7 @@ Gem::Specification.new do |s| s.files = Dir.glob("{lib/**/*}") + %w{ LICENSE README.md Rakefile CHANGELOG.md } s.require_path = 'lib' - s.add_runtime_dependency "activerecord", ">= 6.0" + s.add_runtime_dependency "activerecord", ">= 6.1" s.add_runtime_dependency "railties" s.add_development_dependency "rake" diff --git a/lib/active_snapshot/config.rb b/lib/active_snapshot/config.rb index 4338486..c176da9 100644 --- a/lib/active_snapshot/config.rb +++ b/lib/active_snapshot/config.rb @@ -1,12 +1,24 @@ module ActiveSnapshot class Config - attr_reader :storage_method - def initialize - @storage_method = 'serialized_json' + end + + def storage_method + if @storage_method.nil? + if ActiveSnapshot::SnapshotItem.table_exists? && ActiveSnapshot::SnapshotItem.type_for_attribute(:object).type == :text + # for legacy active_snapshot configurations only + self.storage_method = 'serialized_json' + else + self.storage_method = 'native_json' + end + end + + @storage_method end def storage_method=(value) + # for legacy active_snapshot configurations only + value_str = value.to_s if ['serialized_yaml', 'serialized_json', 'native_json'].include?(value_str) @@ -17,15 +29,15 @@ def storage_method=(value) end def storage_method_yaml? - @storage_method == 'serialized_yaml' + # for legacy active_snapshot configurations only + storage_method == 'serialized_yaml' end - def storage_method_json? - @storage_method == 'serialized_json' + def storage_method_serialized_json? + # for legacy active_snapshot configurations only + storage_method == 'serialized_json' end + alias_method :storage_method_json?, :storage_method_serialized_json? - def storage_method_native_json? - @storage_method == 'native_json' - end end end diff --git a/lib/active_snapshot/models/snapshot.rb b/lib/active_snapshot/models/snapshot.rb index fdfb5ab..043e09d 100644 --- a/lib/active_snapshot/models/snapshot.rb +++ b/lib/active_snapshot/models/snapshot.rb @@ -18,9 +18,11 @@ class Snapshot < ActiveRecord::Base def metadata return @metadata if @metadata - if ActiveSnapshot.config.storage_method_json? + if ActiveSnapshot.config.storage_method_serialized_json? + # for legacy active_snapshot configurations only @metadata = JSON.parse(self[:metadata]) elsif ActiveSnapshot.config.storage_method_yaml? + # for legacy active_snapshot configurations only yaml_method = "unsafe_load" if !YAML.respond_to?("unsafe_load") @@ -28,7 +30,7 @@ def metadata end @metadata = YAML.send(yaml_method, self[:metadata]) - elsif ActiveSnapshot.config.storage_method_native_json? + else @metadata = self[:metadata] end end @@ -36,11 +38,13 @@ def metadata def metadata=(h) @metadata = nil - if ActiveSnapshot.config.storage_method_json? + if ActiveSnapshot.config.storage_method_serialized_json? + # for legacy active_snapshot configurations only self[:metadata] = h.to_json elsif ActiveSnapshot.config.storage_method_yaml? + # for legacy active_snapshot configurations only self[:metadata] = YAML.dump(h) - elsif ActiveSnapshot.config.storage_method_native_json? + else self[:metadata] = h end end diff --git a/lib/active_snapshot/models/snapshot_item.rb b/lib/active_snapshot/models/snapshot_item.rb index 96201b3..17bde7b 100644 --- a/lib/active_snapshot/models/snapshot_item.rb +++ b/lib/active_snapshot/models/snapshot_item.rb @@ -17,27 +17,29 @@ class SnapshotItem < ActiveRecord::Base def object return @object if @object - if ActiveSnapshot.config.storage_method_json? + if ActiveSnapshot.config.storage_method_serialized_json? + # for legacy active_snapshot configurations only @object = self[:object] ? JSON.parse(self[:object]) : {} elsif ActiveSnapshot.config.storage_method_yaml? + # for legacy active_snapshot configurations only yaml_method = YAML.respond_to?(:unsafe_load) ? :unsafe_load : :load @object = self[:object] ? YAML.public_send(yaml_method, self[:object]) : {} - elsif ActiveSnapshot.config.storage_method_native_json? - @object = self[:object] else - raise StandardError, "Unsupported storage_method: `#{ActiveSnapshot.config.storage_method}`" + @object = self[:object] end end def object=(h) @object = nil - if ActiveSnapshot.config.storage_method_json? + if ActiveSnapshot.config.storage_method_serialized_json? + # for legacy active_snapshot configurations only self[:object] = h.to_json elsif ActiveSnapshot.config.storage_method_yaml? + # for legacy active_snapshot configurations only self[:object] = YAML.dump(h) - elsif ActiveSnapshot.config.storage_method_native_json? + else self[:object] = h end end diff --git a/lib/generators/active_snapshot/install/templates/create_snapshots_tables.rb.erb b/lib/generators/active_snapshot/install/templates/create_snapshots_tables.rb.erb index 92a0fee..d087bb8 100644 --- a/lib/generators/active_snapshot/install/templates/create_snapshots_tables.rb.erb +++ b/lib/generators/active_snapshot/install/templates/create_snapshots_tables.rb.erb @@ -8,7 +8,7 @@ class <%= migration_name %> < ActiveRecord::Migration::Current t.string :identifier, index: true t.index [:identifier, :item_id, :item_type], unique: true - t.<%= ActiveSnapshot.config.storage_method == 'native_json' ? 'json' : 'text' %> :metadata + t.json :metadata t.datetime :created_at, null: false end @@ -18,7 +18,7 @@ class <%= migration_name %> < ActiveRecord::Migration::Current t.belongs_to :item, polymorphic: true, null: false, index: true t.index [:snapshot_id, :item_id, :item_type], unique: true - t.<%= ActiveSnapshot.config.storage_method == 'native_json' ? 'json' : 'text' %> :object, null: false + t.json :object, null: false t.datetime :created_at, null: false t.string :child_group_name diff --git a/test/models/snapshot_item_test.rb b/test/models/snapshot_item_test.rb index f0caf0d..6e08afd 100644 --- a/test/models/snapshot_item_test.rb +++ b/test/models/snapshot_item_test.rb @@ -37,8 +37,8 @@ def test_validations assert_equal ["can't be blank"], instance.errors[attr] ### presence error end - shared_post = DATA[:shared_post] - snapshot = shared_post.snapshots.first + post = Post.first! + snapshot = post.snapshots.first instance = @snapshot_item_klass.new(item: snapshot.item, snapshot: snapshot) diff --git a/test/models/snapshot_test.rb b/test/models/snapshot_test.rb index 2389baa..888c441 100644 --- a/test/models/snapshot_test.rb +++ b/test/models/snapshot_test.rb @@ -10,7 +10,7 @@ def teardown end def test_relationships - shared_post = DATA[:shared_post] + shared_post = Post.first! instance = @snapshot_klass.new @@ -38,7 +38,7 @@ def test_relationships end def test_validations - shared_post = DATA[:shared_post] + shared_post = Post.first! snapshot = shared_post.snapshots.first instance = @snapshot_klass.new diff --git a/test/test_helper.rb b/test/test_helper.rb index 934aba4..f7fbeea 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -57,30 +57,12 @@ class ActiveSupport::TestCase ActiveRecord::MigrationContext.new(File.expand_path("dummy_app/db/migrate/", __dir__)).migrate end -require 'rspec/mocks' -module MinitestRSpecMocksIntegration - include RSpec::Mocks::ExampleMethods - - def before_setup - RSpec::Mocks.setup - super - end - - def after_teardown - super - RSpec::Mocks.verify - ensure - RSpec::Mocks.teardown - end -end -Minitest::Test.send(:include, MinitestRSpecMocksIntegration) - -DATA = {}.with_indifferent_access +require "rspec/mocks/minitest_integration" -DATA[:shared_post] = Post.create!(a: 1, b: 3) -DATA[:shared_post].create_snapshot!(identifier: 'v1') -DATA[:shared_post].update_columns(a: 2, b: 4) -DATA[:shared_post].create_snapshot!(identifier: 'v2') +post = Post.create!(a: 1, b: 3) +post.create_snapshot!(identifier: 'v1') +post.update_columns(a: 2, b: 4) +post.create_snapshot!(identifier: 'v2') def assert_time_match(a, b) format = "%d-%m-%Y %h:%M:%S.%L" ### MUST LIMIT THE MILLISECONDS TO 3 decimal places of accuracy, the rest are dropped diff --git a/test/unit/config_test.rb b/test/unit/config_test.rb index 3812666..086619b 100644 --- a/test/unit/config_test.rb +++ b/test/unit/config_test.rb @@ -11,16 +11,30 @@ def teardown ActiveSnapshot.config.storage_method = @orig_storage_method end - def test_defaults_to_serialized_json + def test_defaults_to_serialized_json_if_text_column_exists if ENV["ACTIVE_SNAPSHOT_STORAGE_METHOD"].present? skip end + ActiveSnapshot.config.instance_variable_set("@storage_method", nil) + + allow(ActiveSnapshot::SnapshotItem).to receive(:type_for_attribute).with(:object).and_return(ActiveRecord::Type::Text.new) + assert_equal 'serialized_json', ActiveSnapshot.config.storage_method assert_equal false, ActiveSnapshot.config.storage_method_yaml? - assert_equal true, ActiveSnapshot.config.storage_method_json? - assert_equal false, ActiveSnapshot.config.storage_method_native_json? + assert_equal true, ActiveSnapshot.config.storage_method_serialized_json? + end + + def test_defaults_to_native_json + if ENV["ACTIVE_SNAPSHOT_STORAGE_METHOD"].present? + skip + end + + assert_equal 'native_json', ActiveSnapshot.config.storage_method + + assert_equal false, ActiveSnapshot.config.storage_method_yaml? + assert_equal false, ActiveSnapshot.config.storage_method_serialized_json? end def test_accepts_to_serialized_json @@ -29,8 +43,7 @@ def test_accepts_to_serialized_json assert_equal 'serialized_json', ActiveSnapshot.config.storage_method assert_equal false, ActiveSnapshot.config.storage_method_yaml? - assert_equal true, ActiveSnapshot.config.storage_method_json? - assert_equal false, ActiveSnapshot.config.storage_method_native_json? + assert_equal true, ActiveSnapshot.config.storage_method_serialized_json? end @@ -40,8 +53,7 @@ def test_accepts_serialized_yaml assert_equal 'serialized_yaml', ActiveSnapshot.config.storage_method assert_equal true, ActiveSnapshot.config.storage_method_yaml? - assert_equal false, ActiveSnapshot.config.storage_method_json? - assert_equal false, ActiveSnapshot.config.storage_method_native_json? + assert_equal false, ActiveSnapshot.config.storage_method_serialized_json? end def test_accepts_native_json @@ -50,8 +62,7 @@ def test_accepts_native_json assert_equal "native_json", ActiveSnapshot.config.storage_method, "native_json" assert_equal false, ActiveSnapshot.config.storage_method_yaml? - assert_equal false, ActiveSnapshot.config.storage_method_json? - assert_equal true, ActiveSnapshot.config.storage_method_native_json? + assert_equal false, ActiveSnapshot.config.storage_method_serialized_json? end def test_config_doesnt_accept_not_specified_storage_methods