From 138b18ef1cd76e621856ace93960f6827361d076 Mon Sep 17 00:00:00 2001 From: Nony Dutton Date: Mon, 6 Nov 2023 11:40:40 +0100 Subject: [PATCH] tmp Co-authored-by: Thomas Countz --- .github/workflows/ci.yml | 10 +- lib/property_sets.rb | 14 +- lib/property_sets/active_record_extension.rb | 2 + property_sets.gemspec | 2 +- spec/inheritance_spec.rb | 2 - spec/property_sets_spec.rb | 46 ++-- spec/spec_helper.rb | 16 +- spec/support/account.rb | 61 ----- spec/support/database.rb | 230 ++++++++++++------- spec/support/database.yml | 5 + spec/support/database_setup.rb | 16 ++ spec/support/models.rb | 151 ++++++++++++ spec/support/thing.rb | 6 - 13 files changed, 392 insertions(+), 169 deletions(-) delete mode 100644 spec/support/account.rb create mode 100644 spec/support/database_setup.rb create mode 100644 spec/support/models.rb delete mode 100644 spec/support/thing.rb diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 794461b..4fdad17 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,8 @@ jobs: specs: runs-on: ubuntu-latest + name: Ruby ${{ matrix.ruby-version }}, Rails ${{ matrix.ruby-version }}, LCH ${{ matrix.legacy_connection_handling }} + strategy: fail-fast: false matrix: @@ -17,11 +19,15 @@ jobs: gemfile: - rails6.1 - rails7.0 + legacy_connection_handling: + - 'true' + - 'false' include: - - {ruby-version: '2.7', gemfile: rails6.0} - - {ruby-version: '3.0', gemfile: rails6.0} + - {ruby-version: '2.7', gemfile: rails6.0, legacy_connection_handling: 'true'} + - {ruby-version: '3.0', gemfile: rails6.0, legacy_connection_handling: 'true'} env: BUNDLE_GEMFILE: gemfiles/${{ matrix.gemfile }}.gemfile + LEGACY_CONNECTION_HANDLING: ${{ matrix.legacy_connection_handling }} steps: - uses: zendesk/checkout@v2 - name: Set up Ruby diff --git a/lib/property_sets.rb b/lib/property_sets.rb index 8e58d11..38ada1e 100644 --- a/lib/property_sets.rb +++ b/lib/property_sets.rb @@ -28,10 +28,22 @@ def self.ensure_property_set_class(association, owner_class_name) property_class.owner_assoc = association end - namespace.const_get(const_name.to_s) + klass = namespace.const_get(const_name.to_s) + unless klass.connection_class_for_self == parent_for_property_class(namespace, owner_class_name) + warn "The #{assocation} property set is #{klass.connection_class_for_self} but is now expected \ + to be #{parent_for_property_class(namespace, owner_class_name)}. This is likely because \ + `property_sets_connection_class` was called after the #{association} property set was \ + already created." + end + + klass end def self.parent_for_property_class(namespace, owner_class_name) + if namespace.const_get(owner_class_name).property_sets_connection_class + return namespace.const_get(owner_class_name).property_sets_connection_class + end + namespace.const_get(owner_class_name).connection_class_for_self rescue NameError ::ActiveRecord::Base diff --git a/lib/property_sets/active_record_extension.rb b/lib/property_sets/active_record_extension.rb index 1268910..e77fe41 100644 --- a/lib/property_sets/active_record_extension.rb +++ b/lib/property_sets/active_record_extension.rb @@ -5,6 +5,8 @@ module PropertySets module ActiveRecordExtension module ClassMethods + attr_accessor :property_sets_connection_class + def property_set(association, options = {}, &block) unless include?(PropertySets::ActiveRecordExtension::InstanceMethods) self.send(:prepend, PropertySets::ActiveRecordExtension::InstanceMethods) diff --git a/property_sets.gemspec b/property_sets.gemspec index d135a7d..51e220f 100644 --- a/property_sets.gemspec +++ b/property_sets.gemspec @@ -17,7 +17,7 @@ Gem::Specification.new "property_sets", PropertySets::VERSION do |s| s.add_development_dependency("rake") s.add_development_dependency('actionpack') s.add_development_dependency('rspec') - s.add_development_dependency('byebug') + s.add_development_dependency('pry-byebug') s.files = `git ls-files lib`.split("\n") s.license = "MIT" diff --git a/spec/inheritance_spec.rb b/spec/inheritance_spec.rb index f6392ba..7ac1cb6 100644 --- a/spec/inheritance_spec.rb +++ b/spec/inheritance_spec.rb @@ -2,8 +2,6 @@ require 'active_record' require 'property_sets' -ENV["RAILS_ENV"] = "test" - yaml_config = "spec/support/database.yml" ActiveRecord::Base.configurations = begin YAML.safe_load(IO.read(yaml_config), aliases: true) diff --git a/spec/property_sets_spec.rb b/spec/property_sets_spec.rb index 7a3691d..14d547a 100644 --- a/spec/property_sets_spec.rb +++ b/spec/property_sets_spec.rb @@ -6,6 +6,11 @@ PropertySets::PropertySetModel::COLUMN_TYPE_LIMITS.merge('varchar' => 65535) $-w = old +# RSpec.shared_examples "some example" do |parameter| +# it "construct the container class" do +# end +# end + describe PropertySets do let(:account) { Parent::Account.create(:name => "Name") } let(:relation) { Parent::Account.reflections["settings"] } @@ -33,14 +38,14 @@ it "allow the owner class to be customized" do (Flux = Class.new(ActiveRecord::Base)).property_set(:blot, { - :owner_class_name => 'Foobar' - }) { property :test } + :owner_class_name => 'Foobar' + }) { property :test } expect(defined?(FoobarBlot)).to be_truthy end it "pass-through any options from the second parameter" do - class AnotherThing < ActiveRecord::Base + class AnotherThing < MainDatabase self.table_name = "things" # cheat and reuse things table end @@ -50,6 +55,12 @@ class AnotherThing < ActiveRecord::Base expect(AnotherThing.new.settings.extensions).to include(::Parent::Account::Woot) end +end + +RSpec.shared_examples "different account models" do |account_klass| + let(:account) { account_klass.create(:name => "Name") } + let(:relation) { account_klass.reflections["settings"] } + it "support protecting attributes" do expect(account.settings.protected?(:pro)).to be true expect(account.settings.protected?(:foo)).to be false @@ -111,7 +122,8 @@ class AnotherThing < ActiveRecord::Base end it "reject settings with an invalid name" do - s = Parent::AccountSetting.new(:account => account) + settings_klass = Object.const_get("#{account_klass.to_s}Setting") + s = settings_klass.new(account.model_name.element.to_sym => account) valids = %w(hello hel_lo hell0) + [:hello] invalids = %w(_hello) @@ -157,7 +169,7 @@ class AnotherThing < ActiveRecord::Base it "reference the owner instance when constructing a new record" do record = account.settings.lookup(:baz) expect(record).to be_new_record - expect(record.account.id).to eq(account.id) + expect(record.send(account.model_name.element.to_sym).id).to eq(account.id) end it "reference the owner instance when constructing a new record ...on a new record" do @@ -190,8 +202,8 @@ class AnotherThing < ActiveRecord::Base it "return all property pairs if no arguments are provided" do expect(account.settings.get.keys.sort).to eq( - %w(bar baz bool_false bool_nil bool_nil2 bool_true foo hep pro).sort - ) + %w(bar baz bool_false bool_nil bool_nil2 bool_true foo hep pro).sort + ) end it "ignore non-existent keys" do @@ -259,9 +271,9 @@ class AnotherThing < ActiveRecord::Base expect(account.texts.bar).to eq("0") account.update_attributes!(:texts_attributes => [ - { :id => account.texts.lookup(:foo).id, :name => "foo", :value => "0" }, - { :id => account.texts.lookup(:bar).id, :name => "bar", :value => "1" } - ]) + { :id => account.texts.lookup(:foo).id, :name => "foo", :value => "0" }, + { :id => account.texts.lookup(:bar).id, :name => "bar", :value => "1" } + ]) expect(account.texts.foo).to eq("0") expect(account.texts.bar).to eq("1") @@ -368,7 +380,7 @@ class AnotherThing < ActiveRecord::Base it "creates changed attributes" do account.update_attribute(:old, "it works!") expect(account.previous_changes["old"].last).to eq("it works!") - expect(Parent::Account.find(account.id).old).to eq("it works!") + expect(account_klass.find(account.id).old).to eq("it works!") end it "updates changed attributes for existing property_set data" do @@ -376,7 +388,7 @@ class AnotherThing < ActiveRecord::Base account.save account.update_attribute(:old, "it works!") expect(account.previous_changes["old"].last).to eq("it works!") - expect(Parent::Account.find(account.id).old).to eq("it works!") + expect(account_klass.find(account.id).old).to eq("it works!") end it "updates changed attributes for existing property_set data after set through forwarded method" do @@ -384,7 +396,7 @@ class AnotherThing < ActiveRecord::Base account.save account.update_attribute(:old, "it works!") expect(account.previous_changes["old"].last).to eq("it works!") - expect(Parent::Account.find(account.id).old).to eq("it works!") + expect(account_klass.find(account.id).old).to eq("it works!") end end @@ -510,3 +522,11 @@ class AnotherThing < ActiveRecord::Base end end end + +describe Parent::Account do + it_behaves_like "different account models", Parent::Account +end + +describe Parent::AccountAltDb do + it_behaves_like "different account models", Parent::AccountAltDb +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 093c1da..d9d1f96 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -4,11 +4,23 @@ require 'active_record' require 'active_record/fixtures' +ENV["RAILS_ENV"] = "test" + +LEGACY_CONNECTION_HANDLING = (ENV["LEGACY_CONNECTION_HANDLING"] == "true") + +case "#{ActiveRecord::VERSION::MAJOR}.#{ActiveRecord::VERSION::MINOR}" +when '7.0' + ActiveRecord.legacy_connection_handling = LEGACY_CONNECTION_HANDLING +when '6.1' + ActiveRecord::Base.legacy_connection_handling = LEGACY_CONNECTION_HANDLING +end + require 'property_sets' require 'property_sets/delegator' + +require 'support/database_setup' +require 'support/models' require 'support/database' -require 'support/account' -require 'support/thing' I18n.enforce_available_locales = false diff --git a/spec/support/account.rb b/spec/support/account.rb deleted file mode 100644 index 973d1c9..0000000 --- a/spec/support/account.rb +++ /dev/null @@ -1,61 +0,0 @@ -require_relative 'acts_like_an_integer' - -module Parent - class Account < ActiveRecord::Base - include PropertySets::Delegator - delegate_to_property_set :settings, :old => :hep - - # nonsense module to use in options below, only used as a marker - module Woot # doesn't actually seem to be used in AR4 ? - end - - property_set :settings, extend: Woot do - property :foo - property :bar - property :baz - property :hep, :default => 'skep' - property :pro, :protected => true - property :bool_true, :type => :boolean, :default => true - property :bool_false, :type => :boolean, :default => false - property :bool_nil, :type => :boolean, :default => nil - end - - property_set :settings do - # reopening should maintain `extend` above - property :bool_nil2, :type => :boolean - end - - property_set :texts do - property :foo - property :bar - end - - accepts_nested_attributes_for :texts - - property_set :validations do - property :validated - property :regular - - validates_format_of :value, :with => /\d+/, :message => "BEEP", :if => lambda { |r| r.name.to_sym == :validated } - end - - property_set :typed_data do - property :string_prop, :type => :string - property :datetime_prop, :type => :datetime - property :float_prop, :type => :float - property :int_prop, :type => :integer - property :serialized_prop, :type => :serialized - property :default_prop, :type => :integer, :default => ActsLikeAnInteger.new - property :serialized_prop_with_default, :type => :serialized, :default => "[]" - end - - property_set :tiny_texts do - property :serialized, :type => :serialized - end - end -end - -module Other - class Account < ::Parent::Account - end -end diff --git a/spec/support/database.rb b/spec/support/database.rb index 660369a..0c6a7d2 100644 --- a/spec/support/database.rb +++ b/spec/support/database.rb @@ -3,97 +3,165 @@ ActiveRecord::Base.logger = Logger.new(STDOUT) ActiveRecord::Base.logger.level = Logger::ERROR -config = { - :adapter => "sqlite3", - :database => ":memory:", -} - -# connect and check -ActiveRecord::Base.establish_connection(config) +ActiveRecord::Base.establish_connection(:test_database) ActiveRecord::Base.connection.execute('select 1') ActiveRecord::Migration.verbose = false -ActiveRecord::Schema.define(:version => 1) do - create_table "account_settings", :force => true do |t| - t.integer "account_id" - t.string "name" - t.string "value" - t.datetime "created_at" - t.datetime "updated_at" - end - - add_index :account_settings, [ :account_id, :name ], :unique => true - - create_table "account_texts", :force => true do |t| - t.integer "account_id" - t.string "name" - t.string "value" - t.datetime "created_at" - t.datetime "updated_at" - end - - add_index :account_texts, [ :account_id, :name ], :unique => true - - create_table "account_validations", :force => true do |t| - t.integer "account_id" - t.string "name" - t.string "value" - t.datetime "created_at" - t.datetime "updated_at" +class CreatePrimaryTables < ActiveRecord::Migration[6.0] + def connection + MainDatabase.connection end - add_index :account_validations, [ :account_id, :name ], :unique => true - - create_table "account_typed_data", :force => true do |t| - t.integer "account_id" - t.string "name" - t.string "value" - t.datetime "created_at" - t.datetime "updated_at" + def change + create_table "account_benchmark_settings", :force => true do |t| + t.integer "account_id" + t.string "name" + t.string "value" + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index :account_benchmark_settings, [ :account_id, :name ], :unique => true + + create_table "accounts", :force => true do |t| + t.string "name" + t.datetime "created_at" + t.datetime "updated_at" + end + + create_table "account_alt_dbs", :force => true do |t| + t.string "name" + t.datetime "created_at" + t.datetime "updated_at" + end + + create_table "account_settings", :force => true do |t| + t.integer "account_id" + t.string "name" + t.string "value" + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index :account_settings, [ :account_id, :name ], :unique => true + + create_table "account_texts", :force => true do |t| + t.integer "account_id" + t.string "name" + t.string "value" + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index :account_texts, [ :account_id, :name ], :unique => true + + create_table "account_typed_data", :force => true do |t| + t.integer "account_id" + t.string "name" + t.string "value" + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index :account_typed_data, [ :account_id, :name ], :unique => true + + create_table "account_validations", :force => true do |t| + t.integer "account_id" + t.string "name" + t.string "value" + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index :account_validations, [ :account_id, :name ], :unique => true + + create_table "account_tiny_texts", :force => true do |t| + t.integer "account_id" + t.string "name" + t.text "value", :limit => (2**8 - 1) + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index :account_tiny_texts, [ :account_id, :name ], :unique => true + + create_table "things", :force => true do |t| + t.string "name" + t.datetime "created_at" + t.datetime "updated_at" + end + + create_table "thing_settings", :force => true do |t| + t.integer "thing_id" + t.string "name" + t.string "value" + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index :thing_settings, [ :thing_id, :name ], :unique => true end +end - add_index :account_typed_data, [ :account_id, :name ], :unique => true - - create_table "account_benchmark_settings", :force => true do |t| - t.integer "account_id" - t.string "name" - t.string "value" - t.datetime "created_at" - t.datetime "updated_at" - end - - add_index :account_benchmark_settings, [ :account_id, :name ], :unique => true - - create_table "account_tiny_texts", :force => true do |t| - t.integer "account_id" - t.string "name" - t.text "value", :limit => (2**8 - 1) - t.datetime "created_at" - t.datetime "updated_at" - end - - add_index :account_tiny_texts, [ :account_id, :name ], :unique => true - - create_table "accounts", :force => true do |t| - t.string "name" - t.datetime "created_at" - t.datetime "updated_at" - end +CreatePrimaryTables.migrate(:up) - create_table "things", :force => true do |t| - t.string "name" - t.datetime "created_at" - t.datetime "updated_at" +class CreateAccountSeparateDatabase < ActiveRecord::Migration[6.0] + def connection + AltDatabase.connection end - create_table "thing_settings", :force => true do |t| - t.integer "thing_id" - t.string "name" - t.string "value" - t.datetime "created_at" - t.datetime "updated_at" + def change + create_table "account_alt_db_settings", :force => true do |t| + t.integer "account_alt_db_id" + t.string "name" + t.string "value" + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index :account_alt_db_settings, [ :account_alt_db_id, :name ], :unique => true + + create_table "account_alt_db_texts", :force => true do |t| + t.integer "account_alt_db_id" + t.string "name" + t.string "value" + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index :account_alt_db_texts, [ :account_alt_db_id, :name ], :unique => true + + create_table "account_alt_db_typed_data", :force => true do |t| + t.integer "account_alt_db_id" + t.string "name" + t.string "value" + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index :account_alt_db_typed_data, [ :account_alt_db_id, :name ], :unique => true + + create_table "account_alt_db_validations", :force => true do |t| + t.integer "account_alt_db_id" + t.string "name" + t.string "value" + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index :account_alt_db_validations, [ :account_alt_db_id, :name ], :unique => true + + create_table "account_alt_db_tiny_texts", :force => true do |t| + t.integer "account_alt_db_id" + t.string "name" + t.text "value", :limit => (2**8 - 1) + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index :account_alt_db_tiny_texts, [ :account_alt_db_id, :name ], :unique => true end - - add_index :thing_settings, [ :thing_id, :name ], :unique => true end + +CreateAccountSeparateDatabase.migrate(:up) diff --git a/spec/support/database.yml b/spec/support/database.yml index f2a3df9..4829cf7 100644 --- a/spec/support/database.yml +++ b/spec/support/database.yml @@ -9,3 +9,8 @@ test: encoding: utf8 database: unsharded_database_replica replica: true + + separate_settings_database: + adapter: sqlite3 + encoding: utf8 + database: separate_settings_database diff --git a/spec/support/database_setup.rb b/spec/support/database_setup.rb new file mode 100644 index 0000000..4d7f2ca --- /dev/null +++ b/spec/support/database_setup.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +config = { + :test => { + :test_database => { + :adapter => "sqlite3", + :database => ":memory:", + }, + :test_alt_database => { + :adapter => "sqlite3", + :database => ":memory:" + } + } +} + +ActiveRecord::Base.configurations = config diff --git a/spec/support/models.rb b/spec/support/models.rb new file mode 100644 index 0000000..64c07b9 --- /dev/null +++ b/spec/support/models.rb @@ -0,0 +1,151 @@ +# frozen_string_literal: true +require_relative 'acts_like_an_integer' + +if LEGACY_CONNECTION_HANDLING + class MainDatabase < ActiveRecord::Base + self.abstract_class = true + end + + class AltDatabase < ActiveRecord::Base + self.abstract_class = true + establish_connection(:test_alt_database) + end +else + class MainDatabase < ActiveRecord::Base + self.abstract_class = true + + connects_to(database: {writing: :test_database, reading: :test_database}) + end + + class AltDatabase < ActiveRecord::Base + self.abstract_class = true + + connects_to(database: {writing: :test_alt_database, reading: :test_alt_database}) + end +end + +module Parent + class Account < MainDatabase + include PropertySets::Delegator + + delegate_to_property_set :settings, :old => :hep + + # nonsense module to use in options below, only used as a marker + module Woot # doesn't actually seem to be used in AR4 ? + end + + property_set :settings, extend: Woot do + property :foo + property :bar + property :baz + property :hep, :default => 'skep' + property :pro, :protected => true + property :bool_true, :type => :boolean, :default => true + property :bool_false, :type => :boolean, :default => false + property :bool_nil, :type => :boolean, :default => nil + end + + property_set :settings do + # reopening should maintain `extend` above + property :bool_nil2, :type => :boolean + end + + property_set :texts do + property :foo + property :bar + end + + accepts_nested_attributes_for :texts + + property_set :validations do + property :validated + property :regular + + validates_format_of :value, :with => /\d+/, :message => "BEEP", :if => lambda { |r| r.name.to_sym == :validated } + end + + property_set :typed_data do + property :string_prop, :type => :string + property :datetime_prop, :type => :datetime + property :float_prop, :type => :float + property :int_prop, :type => :integer + property :serialized_prop, :type => :serialized + property :default_prop, :type => :integer, :default => ActsLikeAnInteger.new + property :serialized_prop_with_default, :type => :serialized, :default => "[]" + end + + property_set :tiny_texts do + property :serialized, :type => :serialized + end + end +end + +module Other + class Account < ::Parent::Account + end +end + +module Parent + class AccountAltDb < MainDatabase + include PropertySets::Delegator + + self.property_sets_connection_class = AltDatabase + + delegate_to_property_set :settings, :old => :hep + + # nonsense module to use in options below, only used as a marker + module Woot # doesn't actually seem to be used in AR4 ? + end + + property_set :settings, extend: Woot do + property :foo + property :bar + property :baz + property :hep, :default => 'skep' + property :pro, :protected => true + property :bool_true, :type => :boolean, :default => true + property :bool_false, :type => :boolean, :default => false + property :bool_nil, :type => :boolean, :default => nil + end + + property_set :settings do + # reopening should maintain `extend` above + property :bool_nil2, :type => :boolean + end + + property_set :texts do + property :foo + property :bar + end + + accepts_nested_attributes_for :texts + + property_set :validations do + property :validated + property :regular + + validates_format_of :value, :with => /\d+/, :message => "BEEP", :if => lambda { |r| r.name.to_sym == :validated } + end + + property_set :typed_data do + property :string_prop, :type => :string + property :datetime_prop, :type => :datetime + property :float_prop, :type => :float + property :int_prop, :type => :integer + property :serialized_prop, :type => :serialized + property :default_prop, :type => :integer, :default => ActsLikeAnInteger.new + property :serialized_prop_with_default, :type => :serialized, :default => "[]" + end + + property_set :tiny_texts do + property :serialized, :type => :serialized + end + end +end + +# No delegated property_set +class Thing < MainDatabase + property_set :settings do + property :foo + end +end diff --git a/spec/support/thing.rb b/spec/support/thing.rb deleted file mode 100644 index 8023b94..0000000 --- a/spec/support/thing.rb +++ /dev/null @@ -1,6 +0,0 @@ -# No delegated property_set -class Thing < ActiveRecord::Base - property_set :settings do - property :foo - end -end