Skip to content

Commit

Permalink
Users/amit909sin/issue 195 (#219)
Browse files Browse the repository at this point in the history
* Fix the tenant scoping for delete_all
  • Loading branch information
Amit909Singh authored Dec 3, 2023
1 parent 30accb5 commit fc7ad0d
Show file tree
Hide file tree
Showing 31 changed files with 304 additions and 131 deletions.
5 changes: 4 additions & 1 deletion .github/workflows/active-record-multi-tenant-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,16 @@ jobs:
- rails-6.0
- rails-6.1
- rails-7.0
- rails-7.1
- active-record-6.0
- active-record-6.1
- active-record-7.0
- active-record-7.1
citus_version:
- '10'
- '11'

- '12'

name: Ruby ${{ matrix.ruby }}/${{ matrix.gemfile }} / Citus ${{ matrix.citus_version }}
env:
APPRAISAL: ${{ matrix.appraisal }}
Expand Down
18 changes: 13 additions & 5 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,29 @@ AllCops:
- 'node_modules/**/*'
- 'Vagrantfile'
TargetRubyVersion: 3.0
SuggestExtensions: false
NewCops: enable

Style/FrozenStringLiteralComment:
Gemspec/DevelopmentDependencies:
Enabled: false

Style/Documentation:
Exclude:
- '**/*.rb'
Lint/ConstantDefinitionInBlock:
Enabled: false

Lint/ConstantDefinitionInBlock:
Lint/EmptyBlock:
Enabled: false

Style/ClassAndModuleChildren:
Enabled: false

Style/Documentation:
Exclude:
- '**/*.rb'
Enabled: false

Style/DocumentDynamicEvalDefinition:
Enabled: false

Metrics/BlockLength:
Max: 650

Expand Down
10 changes: 10 additions & 0 deletions Appraisals
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

appraise 'rails-6.0' do
gem 'rails', '~> 6.0.3'
end
Expand All @@ -10,6 +12,10 @@ appraise 'rails-7.0' do
gem 'rails', '~> 7.0.0'
end

appraise 'rails-7.1' do
gem 'rails', '~> 7.1.0'
end

appraise 'active-record-6.0' do
gem 'activerecord', '~> 6.0.3'
end
Expand All @@ -21,3 +27,7 @@ end
appraise 'active-record-7.0' do
gem 'activerecord', '~> 7.0.0'
end

appraise 'active-record-7.1' do
gem 'activerecord', '~> 7.1.0'
end
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,18 @@
# Changelog

## 2.4.0 2023-09-22
* Adds citus 12 to test matrix (#210)
* Adds Support for rails 7.1 (#208)
* Fix missing scope in habtm.rb (#207)
* Update logic inside the tenant_klass_defined? method (#202)

## 2.3.0 2023-06-05
* Adds has_and_belongs_to_many feature with tenant (#193)
* Removes eol ruby versions
* Adds documentation in ReadTheDocs platform (#196)
* Organizes badges in documentation and README.md (#197)
* Wrap ActiveRecord::Base with ActiveSupport.on_load (#199)

## 2.2.0 2022-12-06
* Handle changing tenant from `nil` to a value [#173](https://github.com/citusdata/activerecord-multi-tenant/pull/173)
* Allow Partitioned tables to be created without a primary key [#172](https://github.com/citusdata/activerecord-multi-tenant/pull/172)
Expand Down
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

source 'https://rubygems.org'

gemspec
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# activerecord-multi-tenant [![Build Status](https://github.com/citusdata/activerecord-multi-tenant/actions/workflows/active-record-multi-tenant-tests.yml/badge.svg)](https://github.com/citusdata/activerecord-multi-tenant/actions/workflows/active-record-multi-tenant-tests.yml) [![codecov](https://codecov.io/gh/citusdata/activerecord-multi-tenant/branch/master/graph/badge.svg?token=rw0TsEk4Ld)](https://codecov.io/gh/citusdata/activerecord-multi-tenant) [ ![Gems Version](https://img.shields.io/gem/v/activerecord-multi-tenant.svg)](https://rubygems.org/gems/activerecord-multi-tenant)[ ![Gem Download Count](https://img.shields.io/gem/dt/activerecord-multi-tenant.svg)](https://rubygems.org/gems/activerecord-multi-tenant) [![Documentation Status](https://readthedocs.org/projects/activerecord-multi-tenant/badge/?version=latest)](https://activerecord-multi-tenant.readthedocs.io/en/latest/?badge=latest)
# activerecord-multi-tenant
[![Build Status](https://github.com/citusdata/activerecord-multi-tenant/actions/workflows/active-record-multi-tenant-tests.yml/badge.svg)](https://github.com/citusdata/activerecord-multi-tenant/actions/workflows/active-record-multi-tenant-tests.yml) [![codecov](https://codecov.io/gh/citusdata/activerecord-multi-tenant/branch/master/graph/badge.svg?token=rw0TsEk4Ld)](https://codecov.io/gh/citusdata/activerecord-multi-tenant) [ ![Gems Version](https://img.shields.io/gem/v/activerecord-multi-tenant.svg)](https://rubygems.org/gems/activerecord-multi-tenant)[ ![Gem Download Count](https://img.shields.io/gem/dt/activerecord-multi-tenant.svg)](https://rubygems.org/gems/activerecord-multi-tenant) [![Documentation Status](https://readthedocs.org/projects/activerecord-multi-tenant/badge/?version=latest)](https://activerecord-multi-tenant.readthedocs.io/en/latest/?badge=latest)

Introduction Post: https://www.citusdata.com/blog/2017/01/05/easily-scale-out-multi-tenant-apps/

Expand Down
2 changes: 2 additions & 0 deletions Rakefile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require 'rubygems'
require 'bundler/setup'
require 'bundler/gem_tasks'
Expand Down
8 changes: 5 additions & 3 deletions activerecord-multi-tenant.gemspec
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
# frozen_string_literal: true

$LOAD_PATH.push File.expand_path('lib', __dir__)
require 'activerecord-multi-tenant/version'

Gem::Specification.new do |spec|
spec.name = 'activerecord-multi-tenant'
spec.version = MultiTenant::VERSION
spec.summary = 'ActiveRecord/Rails integration for multi-tenant databases, '\
'in particular the Citus extension for PostgreSQL'
spec.summary = 'ActiveRecord/Rails integration for multi-tenant databases, ' \
'in particular the Citus extension for PostgreSQL'
spec.description = ''
spec.authors = ['Citus Data']
spec.email = '[email protected]'
spec.required_ruby_version = '>= 3.0.0'
spec.metadata = { 'rubygems_mfa_required' => 'true' }

spec.files = `git ls-files`.split("\n")
spec.test_files = `git ls-files -- {spec}/*`.split("\n")
spec.require_paths = ['lib']
spec.homepage = 'https://github.com/citusdata/activerecord-multi-tenant'
spec.license = 'MIT'
Expand Down
4 changes: 2 additions & 2 deletions docs/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ alabaster==0.7.13
# via sphinx
babel==2.12.1
# via sphinx
certifi==2023.5.7
certifi==2023.7.22
# via requests
charset-normalizer==3.1.0
# via requests
Expand Down Expand Up @@ -58,5 +58,5 @@ sphinxcontrib-serializinghtml==1.1.5
# via sphinx
sphinxnotes-strike==1.2
# via -r requirements.in
urllib3==2.0.2
urllib3==2.0.7
# via requests
2 changes: 2 additions & 0 deletions lib/activerecord-multi-tenant/copy_from_client.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module MultiTenant
# Designed to be mixed into an ActiveRecord model to provide
# a copy_from_client method that allows for efficient bulk insertion of
Expand Down
41 changes: 22 additions & 19 deletions lib/activerecord-multi-tenant/delete_operations.rb
Original file line number Diff line number Diff line change
@@ -1,31 +1,34 @@
# frozen_string_literal: true

module Arel
module ActiveRecordRelationExtension
def delete_all(conditions = nil)
tenant_id = MultiTenant.current_tenant_id
tenant_key = MultiTenant.partition_key(MultiTenant.current_tenant_class)
# Overrides the delete_all method to include tenant scoping
def delete_all
# Call the original delete_all method if the current tenant is identified by an ID
return super if MultiTenant.current_tenant_is_id? || MultiTenant.current_tenant.nil?

tenant_key = MultiTenant.partition_key(MultiTenant.current_tenant_class)
tenant_id = MultiTenant.current_tenant_id
arel = eager_loading? ? apply_join_dependency.arel : build_arel
arel.source.left = table

group_values_arel_columns = arel_columns(group_values.uniq)
having_clause_ast = having_clause.ast unless having_clause.empty?
stmt = arel.compile_delete(table[primary_key], having_clause_ast, group_values_arel_columns)

if tenant_id
tenant_condition = table[tenant_key.downcase].eq(tenant_id)
account_condition = table["account_id"].eq(tenant_id)
conditions = Arel::Nodes::And.new([tenant_condition, conditions].compact)
puts "conditions: #{conditions.to_sql}"
puts "tenant_id: #{tenant_id}"
if tenant_id && klass.column_names.include?(tenant_key)
# Check if the tenant key is present in the model's column names
tenant_condition = table[tenant_key].eq(tenant_id)
# Add the tenant condition to the arel query if it is not already present
unless arel.constraints.any? { |node| node.to_sql.include?(tenant_condition.to_sql) }
arel = arel.where(tenant_condition)

Check warning on line 20 in lib/activerecord-multi-tenant/delete_operations.rb

View check run for this annotation

Codecov / codecov/patch

lib/activerecord-multi-tenant/delete_operations.rb#L20

Added line #L20 was not covered by tests
end
end

puts "stmt klass: #{stmt.class}"

if conditions
stmt.where(conditions)
end
subquery = arel.clone
subquery.projections.clear
subquery = subquery.project(table[primary_key])
in_condition = Arel::Nodes::In.new(table[primary_key], subquery.ast)
stmt = Arel::DeleteManager.new.from(table)
stmt.wheres = [in_condition]

puts "stmtt: #{stmt.to_sql}"
# Execute the delete statement using the connection and return the result
klass.connection.delete(stmt, "#{klass} Delete All").tap { reset }
end
end
Expand Down
2 changes: 2 additions & 0 deletions lib/activerecord-multi-tenant/fast_truncate.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# Truncates only the tables that have been modified, according to sequence
# values
# Faster alternative to DatabaseCleaner.clean_with(:truncation, pre_count: true)
Expand Down
11 changes: 5 additions & 6 deletions lib/activerecord-multi-tenant/habtm.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ module ActiveRecord
module Associations
module ClassMethods
# rubocop:disable Naming/PredicateName
def has_and_belongs_to_many_with_tenant(name, options = {}, &extension)
def has_and_belongs_to_many_with_tenant(name, scope = nil, **options, &extension)
# rubocop:enable Naming/PredicateName
has_and_belongs_to_many_without_tenant(name, **options, &extension)
has_and_belongs_to_many_without_tenant(name, scope, **options, &extension)

middle_reflection = _reflections[name.to_s].through_reflection
join_model = middle_reflection.klass
Expand All @@ -34,11 +34,10 @@ def has_and_belongs_to_many_with_tenant(name, options = {}, &extension)

# This method sets the tenant_id on the join table and executes before creation of the join table record.
define_method :tenant_set do
if tenant_enabled
raise MultiTenant::MissingTenantError, 'Tenant Id is not set' unless MultiTenant.current_tenant_id
return unless tenant_enabled
raise MultiTenant::MissingTenantError, 'Tenant Id is not set' unless MultiTenant.current_tenant_id

send("#{tenant_column}=", MultiTenant.current_tenant_id)
end
send("#{tenant_column}=", MultiTenant.current_tenant_id)
end
end
end
Expand Down
23 changes: 11 additions & 12 deletions lib/activerecord-multi-tenant/migrations.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module MultiTenant
module MigrationExtensions
def create_distributed_table(table_name, partition_key)
Expand Down Expand Up @@ -66,22 +68,19 @@ def citus_version

ActiveRecord::Migration.include MultiTenant::MigrationExtensions if defined?(ActiveRecord::Migration)

module ActiveRecord
module ConnectionAdapters # :nodoc:
module SchemaStatements
alias orig_create_table create_table

def create_table(table_name, options = {}, &block)
ret = orig_create_table(table_name, **options.except(:partition_key), &block)
if options[:id] != false && options[:partition_key] && options[:partition_key].to_s != 'id'
execute "ALTER TABLE #{table_name} DROP CONSTRAINT #{table_name}_pkey"
execute "ALTER TABLE #{table_name} ADD PRIMARY KEY(\"#{options[:partition_key]}\", id)"
end
ret
module MultiTenant
module SchemaStatementsExtensions
def create_table(table_name, options = {}, &block)
ret = super(table_name, **options.except(:partition_key), &block)
if options[:id] != false && options[:partition_key] && options[:partition_key].to_s != 'id'
execute "ALTER TABLE #{table_name} DROP CONSTRAINT #{table_name}_pkey"
execute "ALTER TABLE #{table_name} ADD PRIMARY KEY(\"#{options[:partition_key]}\", id)"
end
ret
end
end
end
ActiveRecord::ConnectionAdapters::SchemaStatements.prepend(MultiTenant::SchemaStatementsExtensions)

module ActiveRecord
class SchemaDumper
Expand Down
32 changes: 18 additions & 14 deletions lib/activerecord-multi-tenant/model_extensions.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
require_relative './multi_tenant'
# frozen_string_literal: true

require_relative 'multi_tenant'

module MultiTenant
# Extension to the model to allow scoping of models to the current tenant. This is done by adding
# the multitenant method to the models that need to be scoped. This method is called in the
# model declaration.
# Adds scoped_by_tenant? partition_key, primary_key and inherited methods to the model
module ModelExtensionsClassMethods
DEFAULT_ID_FIELD = 'id'.freeze
DEFAULT_ID_FIELD = 'id'
# executes when multi_tenant method is called in the model. This method adds the following
# methods to the model that calls it.
# scoped_by_tenant? - returns true if the model is scoped by tenant
Expand Down Expand Up @@ -67,7 +69,7 @@ def inherited(subclass)
partition_key = @partition_key

# Create an implicit belongs_to association only if tenant class exists
if MultiTenant.tenant_klass_defined?(tenant_name)
if MultiTenant.tenant_klass_defined?(tenant_name, options)
belongs_to tenant_name, **options.slice(:class_name, :inverse_of, :optional)
.merge(foreign_key: options[:partition_key])
end
Expand All @@ -76,7 +78,7 @@ def inherited(subclass)
after_initialize proc { |record|
if MultiTenant.current_tenant_id &&
(!record.attribute_present?(partition_key) || record.public_send(partition_key.to_sym).nil?)
record.public_send("#{partition_key}=".to_sym, MultiTenant.current_tenant_id)
record.public_send(:"#{partition_key}=", MultiTenant.current_tenant_id)
end
}

Expand All @@ -103,7 +105,7 @@ def inherited(subclass)
tenant_id
end

if MultiTenant.tenant_klass_defined?(tenant_name)
if MultiTenant.tenant_klass_defined?(tenant_name, options)
define_method "#{tenant_name}=" do |model|
super(model)
if send("#{partition_key}_changed?") && persisted? && !send("#{partition_key}_was").nil?
Expand Down Expand Up @@ -188,17 +190,19 @@ def inherited(subclass)
end

# skips statement caching for classes that is Multi-tenant or has a multi-tenant relation
class ActiveRecord::Associations::Association
alias skip_statement_cache_orig skip_statement_cache?
module MultiTenant
module AssociationExtensions
def skip_statement_cache?(*scope)
return true if klass.respond_to?(:scoped_by_tenant?) && klass.scoped_by_tenant?

def skip_statement_cache?(*scope)
return true if klass.respond_to?(:scoped_by_tenant?) && klass.scoped_by_tenant?
if reflection.through_reflection
through_klass = reflection.through_reflection.klass
return true if through_klass.respond_to?(:scoped_by_tenant?) && through_klass.scoped_by_tenant?
end

if reflection.through_reflection
through_klass = reflection.through_reflection.klass
return true if through_klass.respond_to?(:scoped_by_tenant?) && through_klass.scoped_by_tenant?
super(*scope)
end

skip_statement_cache_orig(*scope)
end
end

ActiveRecord::Associations::Association.prepend(MultiTenant::AssociationExtensions)
Loading

0 comments on commit fc7ad0d

Please sign in to comment.