Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Multiple database support #430

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,16 @@ jobs:
- activerecord_6.1
- activerecord_edge
adapter:
- 'sqlite3:///:memory:'
- sqlite3:///tmp/closure_tree_test
- mysql2://root:root@0/closure_tree_test
- postgres://closure_tree:closure_tree@0/closure_tree_test
- postgres://postgres:postgres@0/closure_tree_test
exclude:
- ruby: '3.0'
rails: activerecord_edge

steps:
- name: Checkout
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Setup Ruby
uses: ruby/setup-ruby@v1
Expand All @@ -74,7 +74,8 @@ jobs:
- name: RSpec
env:
RAILS_VERSION: ${{ matrix.rails }}
DB_ADAPTER: ${{ matrix.adapter }}
DATABASE_URL: ${{ matrix.adapter }}
SECONDARY_DATABASE_URL: ${{ matrix.adapter }}_secondary
BUNDLE_GEMFILE: gemfiles/${{ matrix.rails }}.gemfile
WITH_ADVISORY_LOCK_PREFIX: ${{ github.run_id }}
run: bin/rake
9 changes: 5 additions & 4 deletions .github/workflows/ci_jruby.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@ jobs:
- activerecord_7.0
- activerecord_6.1
adapter:
- 'sqlite3:///:memory:'
- sqlite3:///tmp/closure_tree_test
- mysql2://root:root@0/closure_tree_test
- postgres://closure_tree:closure_tree@0/closure_tree_test
- postgres://postgres:postgres@0/closure_tree_test
steps:
- name: Checkout
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Setup Ruby
uses: ruby/setup-ruby@v1
Expand All @@ -63,7 +63,8 @@ jobs:
- name: RSpec
env:
RAILS_VERSION: ${{ matrix.rails }}
DB_ADAPTER: ${{ matrix.adapter }}
DATABASE_URL: ${{ matrix.adapter }}
SECONDARY_DATABASE_URL: ${{ matrix.adapter }}_secondary
BUNDLE_GEMFILE: gemfiles/${{ matrix.rails }}.gemfile
WITH_ADVISORY_LOCK_PREFIX: ${{ github.run_id }}
run: bin/rake
9 changes: 5 additions & 4 deletions .github/workflows/ci_truffleruby.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,13 @@ jobs:
- activerecord_7.0
- activerecord_6.1
adapter:
- 'sqlite3:///:memory:'
- sqlite3:///tmp/closure_tree_test
- mysql2://root:root@0/closure_tree_test
- postgres://closure_tree:closure_tree@0/closure_tree_test
- postgres://postgres:postgres@0/closure_tree_test

steps:
- name: Checkout
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Setup Ruby
uses: ruby/setup-ruby@v1
Expand All @@ -66,7 +66,8 @@ jobs:
- name: RSpec
env:
RAILS_VERSION: ${{ matrix.rails }}
DB_ADAPTER: ${{ matrix.adapter }}
DATABASE_URL: ${{ matrix.adapter }}
SECONDARY_DATABASE_URL: ${{ matrix.adapter }}_secondary
BUNDLE_GEMFILE: gemfiles/${{ matrix.rails }}.gemfile
WITH_ADVISORY_LOCK_PREFIX: ${{ github.run_id }}
run: bin/rake
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@ tmp/
.ruby-*
*.iml
coverage/
.env
4 changes: 0 additions & 4 deletions lib/closure_tree/has_closure_tree.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ def has_closure_tree(options = {})

include ClosureTree::DeterministicOrdering if _ct.order_option?
include ClosureTree::NumericDeterministicOrdering if _ct.order_is_numeric?

connection_pool.release_connection
rescue StandardError => e
raise e unless ClosureTree.configuration.database_less
end

alias_method :acts_as_tree, :has_closure_tree
Expand Down
2 changes: 0 additions & 2 deletions lib/closure_tree/has_closure_tree_root.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,6 @@ def has_closure_tree_root(assoc_name, options = {})

@closure_tree_roots[assoc_name][assoc_map] = root
end

connection_pool.release_connection
end
end
end
78 changes: 40 additions & 38 deletions lib/closure_tree/numeric_order_support.rb
Original file line number Diff line number Diff line change
@@ -1,53 +1,42 @@
module ClosureTree
module NumericOrderSupport

def self.adapter_for_connection(connection)
das = WithAdvisoryLock::DatabaseAdapterSupport.new(connection)
if das.postgresql?
::ClosureTree::NumericOrderSupport::PostgreSQLAdapter
elsif das.mysql?
::ClosureTree::NumericOrderSupport::MysqlAdapter
else
::ClosureTree::NumericOrderSupport::GenericAdapter
end
end

module MysqlAdapter
def reorder_with_parent_id(parent_id, minimum_sort_order_value = nil)
return if parent_id.nil? && dont_order_roots
module_function def reorder_with_parent_id(ct, parent_id, minimum_sort_order_value = nil)
return if parent_id.nil? && ct.dont_order_roots

min_where = if minimum_sort_order_value
"AND #{quoted_order_column} >= #{minimum_sort_order_value}"
"AND #{ct.quoted_order_column} >= #{minimum_sort_order_value}"
else
""
end
connection.execute 'SET @i = 0'
connection.execute <<-SQL.squish
UPDATE #{quoted_table_name}
SET #{quoted_order_column} = (@i := @i + 1) + #{minimum_sort_order_value.to_i - 1}
WHERE #{where_eq(parent_column_name, parent_id)} #{min_where}
ORDER BY #{nulls_last_order_by}
ct.connection.execute 'SET @i = 0'
ct.connection.execute <<-SQL.squish
UPDATE #{ct.quoted_table_name}
SET #{ct.quoted_order_column} = (@i := @i + 1) + #{minimum_sort_order_value.to_i - 1}
WHERE #{ct.where_eq(ct.parent_column_name, parent_id)} #{min_where}
ORDER BY #{ct.nulls_last_order_by}
SQL
end
end

module PostgreSQLAdapter
def reorder_with_parent_id(parent_id, minimum_sort_order_value = nil)
return if parent_id.nil? && dont_order_roots
module_function def reorder_with_parent_id(ct, parent_id, minimum_sort_order_value = nil)
return if parent_id.nil? && ct.dont_order_roots
min_where = if minimum_sort_order_value
"AND #{quoted_order_column} >= #{minimum_sort_order_value}"
"AND #{ct.quoted_order_column} >= #{minimum_sort_order_value}"
else
""
end
connection.execute <<-SQL.squish
UPDATE #{quoted_table_name}
SET #{quoted_order_column(false)} = t.seq + #{minimum_sort_order_value.to_i - 1}
ct.connection.execute <<-SQL.squish
UPDATE #{ct.quoted_table_name}
SET #{ct.quoted_order_column(false)} = t.seq + #{minimum_sort_order_value.to_i - 1}
FROM (
SELECT #{quoted_id_column_name} AS id, row_number() OVER(ORDER BY #{order_by}) AS seq
FROM #{quoted_table_name}
WHERE #{where_eq(parent_column_name, parent_id)} #{min_where}
SELECT #{ct.quoted_id_column_name} AS id, row_number() OVER(ORDER BY #{ct.order_by}) AS seq
FROM #{ct.quoted_table_name}
WHERE #{ct.where_eq(ct.parent_column_name, parent_id)} #{min_where}
) AS t
WHERE #{quoted_table_name}.#{quoted_id_column_name} = t.id and
#{quoted_table_name}.#{quoted_order_column(false)} is distinct from t.seq + #{minimum_sort_order_value.to_i - 1}
WHERE #{ct.quoted_table_name}.#{ct.quoted_id_column_name} = t.id and
#{ct.quoted_table_name}.#{ct.quoted_order_column(false)} is distinct from t.seq + #{minimum_sort_order_value.to_i - 1}
SQL
end

Expand All @@ -57,18 +46,31 @@ def rows_updated(result)
end

module GenericAdapter
def reorder_with_parent_id(parent_id, minimum_sort_order_value = nil)
return if parent_id.nil? && dont_order_roots
scope = model_class.
where(parent_column_sym => parent_id).
order(nulls_last_order_by)
module_function def reorder_with_parent_id(ct, parent_id, minimum_sort_order_value = nil)
return if parent_id.nil? && ct.dont_order_roots
binding.irb
scope = ct.
where(ct.parent_column_sym => parent_id).
order(ct.nulls_last_order_by)
if minimum_sort_order_value
scope = scope.where("#{quoted_order_column} >= #{minimum_sort_order_value}")
scope = scope.where("#{ct.quoted_order_column} >= #{minimum_sort_order_value}")
end
scope.each_with_index do |ea, idx|
ea.update_order_value(idx + minimum_sort_order_value.to_i)
end
end
end


module_function def adapter_for_connection(ct, parent_id, minimum_sort_order_value = nil)
das = WithAdvisoryLock::DatabaseAdapterSupport.new(ct.connection)
if das.postgresql?
PostgreSQLAdapter.reorder_with_parent_id(ct, parent_id, minimum_sort_order_value = nil)
elsif das.mysql?
MysqlAdapter.reorder_with_parent_id(ct, parent_id, minimum_sort_order_value = nil)
else
GenericAdapter.reorder_with_parent_id(ct, parent_id, minimum_sort_order_value = nil)
end
end
end
end
7 changes: 4 additions & 3 deletions lib/closure_tree/support.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ def initialize(model_class, options)
:numeric_order => false
}.merge(options)
raise ArgumentError, "name_column can't be 'path'" if options[:name_column] == 'path'
if order_is_numeric?
extend NumericOrderSupport.adapter_for_connection(connection)
end
end

def hierarchy_class_for_model
Expand All @@ -54,6 +51,10 @@ def hash
hierarchy_class
end

def reorder_with_parent_id(parent_id, minimum_sort_order_value = nil)
NumericOrderSupport.adapter_for_connection(self, parent_id, minimum_sort_order_value)
end

def hierarchy_table_name
# We need to use the table_name, not something like ct_class.to_s.demodulize + "_hierarchies",
# because they may have overridden the table name, which is what we want to be consistent with
Expand Down
2 changes: 1 addition & 1 deletion lib/closure_tree/support_attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def quoted_order_column(include_table_name = true)

# table_name alias keyword , like "AS". When used on table name alias, Oracle Database don't support used 'AS'
def t_alias_keyword
(ActiveRecord::Base.connection.adapter_name.to_sym == :OracleEnhanced) ? "" : "AS"
(connection.adapter_name.to_sym == :OracleEnhanced) ? "" : "AS"
end
end
end
43 changes: 23 additions & 20 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# frozen_string_literal: true


require 'database_cleaner'
require 'closure_tree/test/matcher'
require 'tmpdir'
Expand All @@ -11,6 +10,7 @@
require 'active_record'
require 'active_support/core_ext/array'

puts "Using ActiveRecord #{ActiveRecord.gem_version} and #{RUBY_ENGINE} #{RUBY_ENGINE_VERSION} as #{RUBY_VERSION}"

# Start Simplecov
if RUBY_ENGINE == 'ruby'
Expand All @@ -20,25 +20,33 @@
end
end

primary_database_url = ENV['DATABASE_URL'].presence || "sqlite3:///tmp/closure_tree_test"
secondary_database_url = ENV['SECONDARY_DATABASE_URL'].presence || "sqlite3:///tmp/closure_tree_test-s"

puts "Using primary database #{primary_database_url}"
puts "Using secondary database #{secondary_database_url}"

ActiveRecord::Base.configurations = {
default_env: {
url: ENV.fetch('DATABASE_URL', "sqlite3::memory:"),
properties: { allowPublicKeyRetrieval: true } # for JRuby madness
primary: {
primary: true,
url: primary_database_url,
properties: { allowPublicKeyRetrieval: true } # for JRuby madness
},
secondary: {
url: secondary_database_url,
properties: { allowPublicKeyRetrieval: true } # for JRuby madness
}
}
}

# Configure ActiveRecord
ActiveRecord::Migration.verbose = false
ActiveRecord::Base.table_name_prefix = ENV['DB_PREFIX'].to_s
ActiveRecord::Base.table_name_suffix = ENV['DB_SUFFIX'].to_s
ActiveRecord::Base.establish_connection

def env_db
@env_db ||= if ActiveRecord::Base.respond_to?(:connection_db_config)
ActiveRecord::Base.connection_db_config.adapter
else
ActiveRecord::Base.connection_config[:adapter]
end.to_sym
@env_db ||= ActiveRecord::Base.connection_db_config.adapter.to_sym
end

# Use in specs to skip some tests
Expand Down Expand Up @@ -73,15 +81,12 @@ def sqlite?
# disable monkey patching
# see: https://relishapp.com/rspec/rspec-core/v/3-8/docs/configuration/zero-monkey-patching-mode
config.disable_monkey_patching!
config.before(:suite) do
ENV['FLOCK_DIR'] = Dir.mktmpdir if sqlite?
end

if sqlite?
config.before(:suite) do
ENV['FLOCK_DIR'] = Dir.mktmpdir
end

config.after(:suite) do
FileUtils.remove_entry_secure ENV['FLOCK_DIR']
end
config.after(:suite) do
FileUtils.remove_entry_secure(ENV['FLOCK_DIR']) if sqlite?
end
end

Expand All @@ -92,14 +97,12 @@ def sqlite?
# See: https://github.com/ClosureTree/with_advisory_lock
ENV['WITH_ADVISORY_LOCK_PREFIX'] ||= SecureRandom.hex

ActiveRecord::Base.connection.recreate_database("closure_tree_test") unless sqlite?
puts "Testing with #{env_db} database, ActiveRecord #{ActiveRecord.gem_version} and #{RUBY_ENGINE} #{RUBY_ENGINE_VERSION} as #{RUBY_VERSION}"
# Require our gem
require 'closure_tree'

# Load test helpers
require_relative 'support/schema'
require_relative 'support/models'
require_relative 'support/helpers'
require_relative 'support/exceed_query_limit'
require_relative 'support/query_counter'
puts "Testing with #{env_db} database"
4 changes: 3 additions & 1 deletion spec/support/exceed_query_limit.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# Derived from http://stackoverflow.com/a/13423584/153896. Updated for RSpec 3.
RSpec::Matchers.define :exceed_query_limit do |expected|
supports_block_expectations
Expand All @@ -6,7 +8,7 @@
query_count(&block) > expected
end

failure_message_when_negated do |actual|
failure_message_when_negated do |_actual|
"Expected to run maximum #{expected} queries, got #{@counter.query_count}"
end

Expand Down
4 changes: 3 additions & 1 deletion spec/support/helpers.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
# frozen_string_literal: true

# See http://stackoverflow.com/a/22388177/1268016
def count_queries(&block)
count = 0
counter_fn = ->(name, started, finished, unique_id, payload) do
counter_fn = lambda do |_name, _started, _finished, _unique_id, payload|
count += 1 unless %w[CACHE SCHEMA].include? payload[:name]
end
ActiveSupport::Notifications.subscribed(counter_fn, 'sql.active_record', &block)
Expand Down
Loading
Loading